From c30fa16d4b2f0a303cfd9308219ecad520125095 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 18:16:37 -0300
Subject: [PATCH 01/32] CTxMemPool::removeForBlock now uses RemoveStaged

Coming from btc@7659438a63ef162b4a4f942f86683ae6785f8162
---
 src/txmempool.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index f181d464bcb3..286ab61ba105 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -603,8 +603,12 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
             entries.push_back(*i);
     }
     for (const auto& tx : vtx) {
-        std::list<CTransactionRef> dummy;
-        remove(*tx, dummy, false);
+        txiter it = mapTx.find(tx->GetHash());
+        if (it != mapTx.end()) {
+            setEntries stage;
+            stage.insert(it);
+            RemoveStaged(stage);
+        }
         removeConflicts(*tx, conflicts);
         ClearPrioritisation(tx->GetHash());
     }

From ba323753f24cfa3b7328f3e83fab8e4649d0e083 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 18:21:46 -0300
Subject: [PATCH 02/32] Rename CTxMemPool::remove -> removeRecursive  remove is
 no longer called non-recursively, so simplify the logic and eliminate an
 unnecessary parameter

Coming from btc@5de2baa138cda501038a4558bc169b2cfe5b7d6b
---
 src/test/mempool_tests.cpp | 20 ++++++++++----------
 src/txmempool.cpp          | 22 +++++++++-------------
 src/txmempool.h            |  2 +-
 src/validation.cpp         |  2 +-
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 0a863a6a97c3..7d494f844f2f 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -56,12 +56,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     std::list<CTransactionRef> removed;
 
     // Nothing in pool, remove should do nothing:
-    testPool.remove(txParent, removed, true);
+    testPool.removeRecursive(txParent, removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
 
     // Just the parent:
     testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
-    testPool.remove(txParent, removed, true);
+    testPool.removeRecursive(txParent, removed);
     BOOST_CHECK_EQUAL(removed.size(), 1);
     removed.clear();
     
@@ -73,16 +73,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
         testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
     }
     // Remove Child[0], GrandChild[0] should be removed:
-    testPool.remove(txChild[0], removed, true);
+    testPool.removeRecursive(txChild[0], removed);
     BOOST_CHECK_EQUAL(removed.size(), 2);
     removed.clear();
     // ... make sure grandchild and child are gone:
-    testPool.remove(txGrandChild[0], removed, true);
+    testPool.removeRecursive(txGrandChild[0], removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
-    testPool.remove(txChild[0], removed, true);
+    testPool.removeRecursive(txChild[0], removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
     // Remove parent, all children/grandchildren should go:
-    testPool.remove(txParent, removed, true);
+    testPool.removeRecursive(txParent, removed);
     BOOST_CHECK_EQUAL(removed.size(), 5);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
     removed.clear();
@@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     }
     // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be
     // put into the mempool (maybe because it is non-standard):
-    testPool.remove(txParent, removed, true);
+    testPool.removeRecursive(txParent, removed);
     BOOST_CHECK_EQUAL(removed.size(), 6);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
     removed.clear();
@@ -280,11 +280,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
 
     // Now try removing tx10 and verify the sort order returns to normal
     std::list<CTransactionRef> removed;
-    pool.remove(pool.mapTx.find(tx10.GetHash())->GetTx(), removed, true);
+    pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed);
     CheckSort<descendant_score>(pool, snapshotOrder);
 
-    pool.remove(pool.mapTx.find(tx9.GetHash())->GetTx(), removed, true);
-    pool.remove(pool.mapTx.find(tx8.GetHash())->GetTx(), removed, true);
+    pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed);
+    pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed);
     /* Now check the sort on the mining score index.
      * Final order should be:
      *
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 286ab61ba105..d12f793e3e2c 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -468,7 +468,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
     }
 }
 
-void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>& removed, bool fRecursive)
+void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransactionRef>& removed)
 {
     // Remove transaction from memory pool
     {
@@ -477,8 +477,8 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>&
         txiter origit = mapTx.find(origTx.GetHash());
         if (origit != mapTx.end()) {
             txToRemove.insert(origit);
-        } else if (fRecursive) {
-            // If recursively removing but origTx isn't in the mempool
+        } else {
+            // When recursively removing but origTx isn't in the mempool
             // be sure to remove any children that are in the pool. This can
             // happen during chain re-orgs if origTx isn't re-accepted into
             // the mempool for any reason.
@@ -492,12 +492,8 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>&
             }
         }
         setEntries setAllRemoves;
-        if (fRecursive) {
-            for (const txiter& it : txToRemove) {
-                CalculateDescendants(it, setAllRemoves);
-            }
-        } else {
-            setAllRemoves.swap(txToRemove);
+        for (const txiter& it : txToRemove) {
+            CalculateDescendants(it, setAllRemoves);
         }
         for (const txiter& it : setAllRemoves) {
             removed.emplace_back(it->GetSharedTx());
@@ -532,7 +528,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
     }
     for (const CTransaction& tx : transactionsToRemove) {
         std::list<CTransactionRef> removed;
-        remove(tx, removed, true);
+        removeRecursive(tx, removed);
     }
 }
 
@@ -557,7 +553,7 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
     }
     for (const CTransaction& tx : transactionsToRemove) {
         std::list<CTransactionRef> removed;
-        remove(tx, removed, true);
+        removeRecursive(tx, removed);
     }
 }
 
@@ -571,7 +567,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
         if (it != mapNextTx.end()) {
             const CTransaction& txConflict = *it->second.ptx;
             if (txConflict != tx) {
-                remove(txConflict, removed, true);
+                removeRecursive(txConflict, removed);
             }
         }
     }
@@ -582,7 +578,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
             if (it != mapSaplingNullifiers.end()) {
                 const CTransaction& txConflict = *it->second;
                 if (txConflict != tx) {
-                    remove(txConflict, removed, true);
+                    removeRecursive(txConflict, removed);
                 }
             }
         }
diff --git a/src/txmempool.h b/src/txmempool.h
index 898533c3f149..f69b846228b4 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -477,7 +477,7 @@ class CTxMemPool
     // then invoke the second version.
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true);
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
-    void remove(const CTransaction& tx, std::list<CTransactionRef>& removed, bool fRecursive = false);
+    void removeRecursive(const CTransaction& tx, std::list<CTransactionRef>& removed);
     void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags);
     void removeWithAnchor(const uint256& invalidRoot);
     void removeConflicts(const CTransaction& tx, std::list<CTransactionRef>& removed);
diff --git a/src/validation.cpp b/src/validation.cpp
index 43f069db3155..d2feed1c0b04 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1965,7 +1965,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
         std::list<CTransactionRef> removed;
         CValidationState stateDummy;
         if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) {
-            mempool.remove(*tx, removed, true);
+            mempool.removeRecursive(*tx, removed);
         } else if (mempool.exists(tx->GetHash())) {
             vHashUpdate.push_back(tx->GetHash());
         }

From 1fa40ac58ccffa65363bdde742e7294480db4d4e Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 18:32:09 -0300
Subject: [PATCH 03/32] Remove work limit in UpdateForDescendants()  The work
 limit served to prevent the descendant walking algorithm from doing too much
 work by marking the parent transaction as dirty. However to implement
 ancestor tracking, it's not possible to similarly mark those descendant
 transactions as dirty without having to calculate them to begin with.  This
 commit removes the work limit altogether. With appropriate chain limits
 (-limitdescendantcount) the concern about doing too much work inside this
 function should be mitigated.

Coming from btc@76a76321d2f36992178ddaaf4d023c5e33c14fbf
---
 src/txmempool.cpp | 63 ++++++++++-------------------------------------
 src/txmempool.h   | 19 +-------------
 2 files changed, 14 insertions(+), 68 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index d12f793e3e2c..470f1e9ccb5d 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -63,21 +63,13 @@ void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
 // Update the given tx for any in-mempool descendants.
 // Assumes that setMemPoolChildren is correct for the given tx and all
 // descendants.
-bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit, cacheMap &cachedDescendants, const std::set<uint256> &setExclude)
+void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set<uint256> &setExclude)
 {
-    // Track the number of entries (outside setExclude) that we'd need to visit
-    // (will bail out if it exceeds maxDescendantsToVisit)
-    int nChildrenToVisit = 0;
-
     setEntries stageEntries, setAllDescendants;
     stageEntries = GetMemPoolChildren(updateIt);
 
     while (!stageEntries.empty()) {
         const txiter cit = *stageEntries.begin();
-        if (cit->IsDirty()) {
-            // Don't consider any more children if any descendant is dirty
-            return false;
-        }
         setAllDescendants.insert(cit);
         stageEntries.erase(cit);
         const setEntries &setChildren = GetMemPoolChildren(cit);
@@ -87,22 +79,11 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit
                 // We've already calculated this one, just add the entries for this set
                 // but don't traverse again.
                 for (const txiter& cacheEntry : cacheIt->second) {
-                    // update visit count only for new child transactions
-                    // (outside of setExclude and stageEntries)
-                    if (setAllDescendants.insert(cacheEntry).second &&
-                            !setExclude.count(cacheEntry->GetTx().GetHash()) &&
-                            !stageEntries.count(cacheEntry)) {
-                        nChildrenToVisit++;
-                    }
+                    setAllDescendants.insert(cacheEntry);
                 }
             } else if (!setAllDescendants.count(childEntry)) {
-                // Schedule for later processing and update our visit count
-                if (stageEntries.insert(childEntry).second && !setExclude.count(childEntry->GetTx().GetHash())) {
-                        nChildrenToVisit++;
-                }
-            }
-            if (nChildrenToVisit > maxDescendantsToVisit) {
-                return false;
+                // Schedule for later processing
+                stageEntries.insert(childEntry);
             }
         }
     }
@@ -120,7 +101,6 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit
         }
     }
     mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
-    return true;
 }
 
 // vHashesToUpdate is the set of transaction hashes from a disconnected block
@@ -166,10 +146,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
                 UpdateParent(childIter, it, true);
             }
         }
-        if (!UpdateForDescendants(it, 100, mapMemPoolDescendantsToUpdate, setAlreadyIncluded)) {
-            // Mark as dirty if we can't do the calculation.
-            mapTx.modify(it, set_dirty());
-        }
+        UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded);
     }
 }
 
@@ -300,23 +277,14 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove)
     }
 }
 
-void CTxMemPoolEntry::SetDirty()
-{
-    nCountWithDescendants = 0;
-    nSizeWithDescendants = nTxSize;
-    nFeesWithDescendants = nFee;
-}
-
 void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount)
 {
-    if (!IsDirty()) {
-        nSizeWithDescendants += modifySize;
-        assert(int64_t(nSizeWithDescendants) > 0);
-        nFeesWithDescendants += modifyFee;
-        assert(nFeesWithDescendants >= 0);
-        nCountWithDescendants += modifyCount;
-        assert(int64_t(nCountWithDescendants) > 0);
-    }
+    nSizeWithDescendants += modifySize;
+    assert(int64_t(nSizeWithDescendants) > 0);
+    nFeesWithDescendants += modifyFee;
+    assert(nFeesWithDescendants >= 0);
+    nCountWithDescendants += modifyCount;
+    assert(int64_t(nCountWithDescendants) > 0);
 }
 
 CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) :
@@ -714,13 +682,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             // Also check to make sure size/fees is greater than sum with immediate children.
             // just a sanity check, not definitive that this calc is correct...
             // also check that the size is less than the size of the entire mempool.
-            if (!it->IsDirty()) {
-                assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
-                assert(it->GetFeesWithDescendants() >= childFees + it->GetFee());
-            } else {
-                assert(it->GetSizeWithDescendants() == it->GetTxSize());
-                assert(it->GetFeesWithDescendants() == it->GetFee());
-            }
+            assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
+            assert(it->GetFeesWithDescendants() >= childFees + it->GetFee());
             assert(it->GetFeesWithDescendants() >= 0);
         }
 
diff --git a/src/txmempool.h b/src/txmempool.h
index f69b846228b4..a15ac2e5d4fc 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -117,13 +117,6 @@ class CTxMemPoolEntry
     // Updates the fee delta used for mining priority score
     void UpdateFeeDelta(int64_t feeDelta);
 
-    /** We can set the entry to be dirty if doing the full calculation of in-
-     *  mempool descendants will be too expensive, which can potentially happen
-     *  when re-adding transactions from a block back to the mempool.
-     */
-    void SetDirty();
-    bool IsDirty() const { return nCountWithDescendants == 0; }
-
     uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
     uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
     CAmount GetFeesWithDescendants() const { return nFeesWithDescendants; }
@@ -147,12 +140,6 @@ struct update_descendant_state
         int64_t modifyCount;
 };
 
-struct set_dirty
-{
-    void operator() (CTxMemPoolEntry &e)
-        { e.SetDirty(); }
-};
-
 struct update_fee_delta
 {
     update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { }
@@ -607,15 +594,11 @@ class CTxMemPool
      *  updated and hence their state is already reflected in the parent
      *  state).
      *
-     *  If updating an entry requires looking at more than maxDescendantsToVisit
-     *  transactions, outside of the ones in setExclude, then give up.
-     *
      *  cachedDescendants will be updated with the descendants of the transaction
      *  being updated, so that future invocations don't need to walk the
      *  same transaction again, if encountered in another transaction chain.
      */
-    bool UpdateForDescendants(txiter updateIt,
-            int maxDescendantsToVisit,
+    void UpdateForDescendants(txiter updateIt,
             cacheMap &cachedDescendants,
             const std::set<uint256> &setExclude);
     /** Update ancestors of hash to add/remove it as a descendant transaction. */

From 8325bb445812aeef0eb1df92a9a03251a8ec3913 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 18:53:37 -0300
Subject: [PATCH 04/32] Fix mempool limiting for PrioritiseTransaction Redo the
 feerate index to be based on mining score, rather than fee.

Update mempool_packages.py to test prioritisetransaction's effect on
package scores.

Coming from btc@eb306664e786ae43d539fde66f0fbe2a3e89d910
---
 src/rpc/blockchain.cpp              |  4 +--
 src/txmempool.cpp                   | 49 ++++++++++++++++-------------
 src/txmempool.h                     | 43 ++++++++++++-------------
 test/functional/mempool_packages.py | 24 ++++++++++++++
 4 files changed, 76 insertions(+), 44 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 03eea4e0a419..ab699df23af5 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -399,7 +399,7 @@ UniValue mempoolToJSON(bool fVerbose = false)
             info.pushKV("currentpriority", e.GetPriority(chainActive.Height()));
             info.pushKV("descendantcount", e.GetCountWithDescendants());
             info.pushKV("descendantsize", e.GetSizeWithDescendants());
-            info.pushKV("descendantfees", e.GetFeesWithDescendants());
+            info.pushKV("descendantfees", e.GetModFeesWithDescendants());
             const CTransaction& tx = e.GetTx();
             std::set<std::string> setDepends;
             for (const CTxIn& txin : tx.vin) {
@@ -456,7 +456,7 @@ UniValue getrawmempool(const JSONRPCRequest& request)
             "    \"currentpriority\" : n,  (numeric) transaction priority now\n"
             "    \"descendantcount\" : n,  (numeric) number of in-mempool descendant transactions (including this one)\n"
             "    \"descendantsize\" : n,   (numeric) size of in-mempool descendants (including this one)\n"
-            "    \"descendantfees\" : n,   (numeric) fees of in-mempool descendants (including this one)\n"
+            "    \"descendantfees\" : n,   (numeric) modified fees (see above) of in-mempool descendants (including this one)\n"
             "    \"depends\" : [           (array) unconfirmed transactions used as inputs for this transaction\n"
             "        \"transactionid\",    (string) parent transaction id\n"
             "       ... ]\n"
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 470f1e9ccb5d..95a1d3a62cd6 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -33,7 +33,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe
 
     nCountWithDescendants = 1;
     nSizeWithDescendants = nTxSize;
-    nFeesWithDescendants = nFee;
+    nModFeesWithDescendants = nFee;
     CAmount nValueIn = _tx->GetValueOut()+nFee;
     assert(inChainInputValue <= nValueIn);
 
@@ -57,6 +57,7 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
 
 void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
 {
+    nModFeesWithDescendants += newFeeDelta - feeDelta;
     feeDelta = newFeeDelta;
 }
 
@@ -95,7 +96,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan
     for (const txiter& cit : setAllDescendants) {
         if (!setExclude.count(cit->GetTx().GetHash())) {
             modifySize += cit->GetTxSize();
-            modifyFee += cit->GetFee();
+            modifyFee += cit->GetModifiedFee();
             modifyCount++;
             cachedDescendants[updateIt].insert(cit);
         }
@@ -221,7 +222,7 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors
     }
     const int64_t updateCount = (add ? 1 : -1);
     const int64_t updateSize = updateCount * it->GetTxSize();
-    const CAmount updateFee = updateCount * it->GetFee();
+    const CAmount updateFee = updateCount * it->GetModifiedFee();
     for (const txiter& ancestorIt : setAncestors) {
         mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount));
     }
@@ -281,8 +282,7 @@ void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t
 {
     nSizeWithDescendants += modifySize;
     assert(int64_t(nSizeWithDescendants) > 0);
-    nFeesWithDescendants += modifyFee;
-    assert(nFeesWithDescendants >= 0);
+    nModFeesWithDescendants += modifyFee;
     nCountWithDescendants += modifyCount;
     assert(int64_t(nCountWithDescendants) > 0);
 }
@@ -334,6 +334,17 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
     indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
     mapLinks.insert(make_pair(newit, TxLinks()));
 
+    // Update transaction for any feeDelta created by PrioritiseTransaction
+    // TODO: refactor so that the fee delta is calculated before inserting
+    // into mapTx.
+    std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
+    if (pos != mapDeltas.end()) {
+        const std::pair<double, CAmount> &deltas = pos->second;
+        if (deltas.second) {
+            mapTx.modify(newit, update_fee_delta(deltas.second));
+        }
+    }
+
     // Update cachedInnerUsage to include contained transaction's usage.
     // (When we update the entry for in-mempool parents, memory usage will be
     // further updated.)
@@ -363,15 +374,6 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
     }
     UpdateAncestorsOf(true, newit, setAncestors);
 
-    // Update transaction's score for any feeDelta created by PrioritiseTransaction
-    std::map<uint256, std::pair<double, CAmount> >::const_iterator pos = mapDeltas.find(hash);
-    if (pos != mapDeltas.end()) {
-        const std::pair<double, CAmount> &deltas = pos->second;
-        if (deltas.second) {
-            mapTx.modify(newit, update_fee_delta(deltas.second));
-        }
-    }
-
     // Save spent nullifiers
     if (tx.IsShieldedTx()) {
         for (const SpendDescription& sd : tx.sapData->vShieldedSpend) {
@@ -669,22 +671,19 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             CTxMemPool::setEntries setChildrenCheck;
             std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(tx.GetHash(), 0));
             int64_t childSizes = 0;
-            CAmount childFees = 0;
+            CAmount childModFee = 0;
             for (; iter != mapNextTx.end() && iter->first.hash == tx.GetHash(); ++iter) {
                 txiter childit = mapTx.find(iter->second.ptx->GetHash());
                 assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
                 if (setChildrenCheck.insert(childit).second) {
                     childSizes += childit->GetTxSize();
-                    childFees += childit->GetFee();
+                    childModFee += childit->GetModifiedFee();
                 }
             }
             assert(setChildrenCheck == GetMemPoolChildren(it));
-            // Also check to make sure size/fees is greater than sum with immediate children.
+            // Also check to make sure size is greater than sum with immediate children.
             // just a sanity check, not definitive that this calc is correct...
-            // also check that the size is less than the size of the entire mempool.
             assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
-            assert(it->GetFeesWithDescendants() >= childFees + it->GetFee());
-            assert(it->GetFeesWithDescendants() >= 0);
         }
 
         if (fDependsWait)
@@ -831,6 +830,14 @@ void CTxMemPool::PrioritiseTransaction(const uint256 hash, const std::string str
         txiter it = mapTx.find(hash);
         if (it != mapTx.end()) {
             mapTx.modify(it, update_fee_delta(deltas.second));
+            // Now update all ancestors' modified fees with descendants
+            setEntries setAncestors;
+            uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
+            std::string dummy;
+            CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
+            for (const txiter& ancestorIt : setAncestors) {
+                mapTx.modify(ancestorIt, update_descendant_state(0, nFeeDelta, 0));
+            }
         }
     }
     LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", strHash, dPriorityDelta, FormatMoney(nFeeDelta));
@@ -1030,7 +1037,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
         // "minimum reasonable fee rate" (ie some value under which we consider txn
         // to have 0 fee). This way, we don't allow txn to enter mempool with feerate
         // equal to txn which were removed with no block in between.
-        CFeeRate removed(it->GetFeesWithDescendants(), it->GetSizeWithDescendants());
+        CFeeRate removed(it->GetModFeesWithDescendants(), it->GetSizeWithDescendants());
         removed += minReasonableRelayFee;
         trackPackageRemoved(removed);
         maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed);
diff --git a/src/txmempool.h b/src/txmempool.h
index a15ac2e5d4fc..79b9773526e8 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -49,12 +49,12 @@ class CTxMemPool;
  * ("descendant" transactions).
  *
  * When a new entry is added to the mempool, we update the descendant state
- * (nCountWithDescendants, nSizeWithDescendants, and nFeesWithDescendants) for
+ * (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for
  * all ancestors of the newly added transaction.
  *
  * If updating the descendant state is skipped, we can mark the entry as
- * "dirty", and set nSizeWithDescendants/nFeesWithDescendants to equal nTxSize/
- * nTxFee. (This can potentially happen during a reorg, where we limit the
+ * "dirty", and set nSizeWithDescendants/nModFeesWithDescendants to equal nTxSize/
+ * nFee+feeDelta. (This can potentially happen during a reorg, where we limit the
  * amount of work we're willing to do to avoid consuming too much CPU.)
  *
  */
@@ -81,11 +81,11 @@ class CTxMemPoolEntry
     // Information about descendants of this transaction that are in the
     // mempool; if we remove this transaction we must remove all of these
     // descendants as well.  if nCountWithDescendants is 0, treat this entry as
-    // dirty, and nSizeWithDescendants and nFeesWithDescendants will not be
+    // dirty, and nSizeWithDescendants and nModFeesWithDescendants will not be
     // correct.
     uint64_t nCountWithDescendants; //! number of descendant transactions
     uint64_t nSizeWithDescendants;  //! ... and size
-    CAmount nFeesWithDescendants;  //! ... and total fees (all including us)
+    CAmount nModFeesWithDescendants;  //! ... and total fees (all including us)
 
 public:
     CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee,
@@ -114,12 +114,13 @@ class CTxMemPoolEntry
 
     // Adjusts the descendant state, if this entry is not dirty.
     void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount);
-    // Updates the fee delta used for mining priority score
+    // Updates the fee delta used for mining priority score, and the
+    // modified fees with descendants.
     void UpdateFeeDelta(int64_t feeDelta);
 
     uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
     uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
-    CAmount GetFeesWithDescendants() const { return nFeesWithDescendants; }
+    CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
 
     bool GetSpendsCoinbaseOrCoinstake() const { return spendsCoinbaseOrCoinstake; }
 };
@@ -160,27 +161,27 @@ struct mempoolentry_txid
     }
 };
 
-/** \class CompareTxMemPoolEntryByFee
+/** \class CompareTxMemPoolEntryByDescendantScore
  *
- *  Sort an entry by max(feerate of entry's tx, feerate with all descendants).
+ *  Sort an entry by max(score/size of entry's tx, score/size with all descendants).
  */
-class CompareTxMemPoolEntryByFee
+class CompareTxMemPoolEntryByDescendantScore
 {
 public:
     bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
     {
-        bool fUseADescendants = UseDescendantFeeRate(a);
-        bool fUseBDescendants = UseDescendantFeeRate(b);
+        bool fUseADescendants = UseDescendantScore(a);
+        bool fUseBDescendants = UseDescendantScore(b);
 
-        double aFees = fUseADescendants ? a.GetFeesWithDescendants() : a.GetFee();
+        double aModFee = fUseADescendants ? a.GetModFeesWithDescendants() : a.GetModifiedFee();
         double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize();
 
-        double bFees = fUseBDescendants ? b.GetFeesWithDescendants() : b.GetFee();
+        double bModFee = fUseBDescendants ? b.GetModFeesWithDescendants() : b.GetModifiedFee();
         double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize();
 
         // Avoid division by rewriting (a/b > c/d) as (a*d > c*b).
-        double f1 = aFees * bSize;
-        double f2 = aSize * bFees;
+        double f1 = aModFee * bSize;
+        double f2 = aSize * bModFee;
 
         if (f1 == f2) {
             return a.GetTime() >= b.GetTime();
@@ -188,11 +189,11 @@ class CompareTxMemPoolEntryByFee
         return f1 < f2;
     }
 
-    // Calculate which feerate to use for an entry (avoiding division).
-    bool UseDescendantFeeRate(const CTxMemPoolEntry &a) const
+    // Calculate which score to use for an entry (avoiding division).
+    bool UseDescendantScore(const CTxMemPoolEntry &a) const
     {
-        double f1 = (double)a.GetFee() * a.GetSizeWithDescendants();
-        double f2 = (double)a.GetFeesWithDescendants() * a.GetTxSize();
+        double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants();
+        double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize();
         return f2 > f1;
     }
 };
@@ -367,7 +368,7 @@ class CTxMemPool
             boost::multi_index::ordered_non_unique<
                 boost::multi_index::tag<descendant_score>,
                 boost::multi_index::identity<CTxMemPoolEntry>,
-                CompareTxMemPoolEntryByFee
+                CompareTxMemPoolEntryByDescendantScore
             >,
             // sorted by entry time
             boost::multi_index::ordered_non_unique<
diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py
index b3fbec58bc1d..14f9df526527 100755
--- a/test/functional/mempool_packages.py
+++ b/test/functional/mempool_packages.py
@@ -69,17 +69,41 @@ def run_test(self):
         for x in reversed(chain):
             assert_equal(mempool[x]['descendantcount'], descendant_count)
             descendant_fees += mempool[x]['fee']
+            assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee'])
             assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees)
             descendant_size += mempool[x]['size']
             assert_equal(mempool[x]['descendantsize'], descendant_size)
             descendant_count += 1
 
+        # Check that descendant modified fees includes fee deltas from
+        # prioritisetransaction
+        self.nodes[0].prioritisetransaction(chain[-1], 0, 1000)
+        mempool = self.nodes[0].getrawmempool(True)
+
+        descendant_fees = 0
+        for x in reversed(chain):
+            descendant_fees += mempool[x]['fee']
+            assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+1000)
+
         # Adding one more transaction on to the chain should fail.
         try:
             self.chain_transaction(self.nodes[0],txid, vout, value, fee, 1)
         except JSONRPCException as e:
             self.log.info("too-long-ancestor-chain successfully rejected")
 
+        # Check that prioritising a tx before it's added to the mempool works
+        self.nodes[0].generate(1)
+        self.nodes[0].prioritisetransaction(chain[-1], 0, 2000)
+        self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash())
+        mempool = self.nodes[0].getrawmempool(True)
+
+        descendant_fees = 0
+        for x in reversed(chain):
+            descendant_fees += mempool[x]['fee']
+            if (x == chain[-1]):
+                assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002))
+            assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+2000)
+
         # TODO: check that node1's mempool is as expected
 
         # TODO: test ancestor size limits

From 64e84e2665419028e09ce11f7ee29a024d1ff7d5 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 19:38:53 -0300
Subject: [PATCH 05/32] Add ancestor tracking to mempool  This implements
 caching of ancestor state to each mempool entry, similar to descendant
 tracking, but also including caching sigops-with-ancestors (as that metric
 will be helpful to future code that implements better transaction selection
 in CreatenewBlock).

Coming from btc@72abd2ce3c5ad8157d3a993693df1919a6ad79c3
---
 src/txmempool.cpp  | 88 ++++++++++++++++++++++++++++++++++++++--------
 src/txmempool.h    | 49 ++++++++++++++++++++++----
 src/validation.cpp |  1 +
 3 files changed, 117 insertions(+), 21 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 95a1d3a62cd6..1b50ee4b7dc4 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -38,6 +38,11 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe
     assert(inChainInputValue <= nValueIn);
 
     feeDelta = 0;
+
+    nCountWithAncestors = 1;
+    nSizeWithAncestors = nTxSize;
+    nModFeesWithAncestors = nFee;
+    nSigOpCountWithAncestors = sigOpCount;
 }
 
 CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other)
@@ -58,6 +63,7 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const
 void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
 {
     nModFeesWithDescendants += newFeeDelta - feeDelta;
+    nModFeesWithAncestors += newFeeDelta - feeDelta;
     feeDelta = newFeeDelta;
 }
 
@@ -99,6 +105,8 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan
             modifyFee += cit->GetModifiedFee();
             modifyCount++;
             cachedDescendants[updateIt].insert(cit);
+            // Update ancestor state for each descendant
+            mapTx.modify(cit, update_ancestor_state(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCount()));
         }
     }
     mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
@@ -108,6 +116,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan
 // which has been re-added to the mempool.
 // for each entry, look for descendants that are outside hashesToUpdate, and
 // add fee/size information for such descendants to the parent.
+// for each such descendant, also update the ancestor state to include the parent.
 void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate)
 {
     LOCK(cs);
@@ -228,6 +237,20 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors
     }
 }
 
+void CTxMemPool::UpdateEntryForAncestors(txiter it, const setEntries &setAncestors)
+{
+    int64_t updateCount = setAncestors.size();
+    int64_t updateSize = 0;
+    CAmount updateFee = 0;
+    int updateSigOps = 0;
+    for (const txiter& ancestorIt : setAncestors) {
+        updateSize += ancestorIt->GetTxSize();
+        updateFee += ancestorIt->GetModifiedFee();
+        updateSigOps += ancestorIt->GetSigOpCount();
+    }
+    mapTx.modify(it, update_ancestor_state(updateSize, updateFee, updateCount, updateSigOps));
+}
+
 void CTxMemPool::UpdateChildrenForRemoval(txiter it)
 {
     const setEntries &setMemPoolChildren = GetMemPoolChildren(it);
@@ -236,11 +259,30 @@ void CTxMemPool::UpdateChildrenForRemoval(txiter it)
     }
 }
 
-void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove)
+void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants)
 {
     // For each entry, walk back all ancestors and decrement size associated with this
     // transaction
     const uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
+    if (updateDescendants) {
+        // updateDescendants should be true whenever we're not recursively
+        // removing a tx and all its descendants, eg when a transaction is
+        // confirmed in a block.
+        // Here we only update statistics and not data in mapLinks (which
+        // we need to preserve until we're finished with all operations that
+        // need to traverse the mempool).
+        for (const txiter& removeIt : entriesToRemove) {
+            setEntries setDescendants;
+            CalculateDescendants(removeIt, setDescendants);
+            setDescendants.erase(removeIt); // don't update state for self
+            int64_t modifySize = -((int64_t)removeIt->GetTxSize());
+            CAmount modifyFee = -removeIt->GetModifiedFee();
+            int modifySigOps = -removeIt->GetSigOpCount();
+            for (const txiter& dit : setDescendants) {
+                mapTx.modify(dit, update_ancestor_state(modifySize, modifyFee, -1, modifySigOps));
+            }
+        }
+    }
     for (const txiter& removeIt : entriesToRemove) {
         setEntries setAncestors;
         const CTxMemPoolEntry &entry = *removeIt;
@@ -264,10 +306,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove)
         // transactions as the set of things to update for removal.
         CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
         // Note that UpdateAncestorsOf severs the child links that point to
-        // removeIt in the entries for the parents of removeIt.  This is
-        // fine since we don't need to use the mempool children of any entries
-        // to walk back over our ancestors (but we do need the mempool
-        // parents!)
+        // removeIt in the entries for the parents of removeIt.
         UpdateAncestorsOf(false, removeIt, setAncestors);
     }
     // After updating all the ancestor sizes, we can now sever the link between each
@@ -278,7 +317,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove)
     }
 }
 
-void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount)
+void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount)
 {
     nSizeWithDescendants += modifySize;
     assert(int64_t(nSizeWithDescendants) > 0);
@@ -287,6 +326,17 @@ void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t
     assert(int64_t(nCountWithDescendants) > 0);
 }
 
+void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps)
+{
+    nSizeWithAncestors += modifySize;
+    assert(int64_t(nSizeWithAncestors) > 0);
+    nModFeesWithAncestors += modifyFee;
+    nCountWithAncestors += modifyCount;
+    assert(int64_t(nCountWithAncestors) > 0);
+    nSigOpCountWithAncestors += modifySigOps;
+    assert(int(nSigOpCountWithAncestors) >= 0);
+}
+
 CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) :
         nTransactionsUpdated(0)
 {
@@ -373,6 +423,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
         }
     }
     UpdateAncestorsOf(true, newit, setAncestors);
+    UpdateEntryForAncestors(newit, setAncestors);
 
     // Save spent nullifiers
     if (tx.IsShieldedTx()) {
@@ -468,7 +519,7 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransact
         for (const txiter& it : setAllRemoves) {
             removed.emplace_back(it->GetSharedTx());
         }
-        RemoveStaged(setAllRemoves);
+        RemoveStaged(setAllRemoves, false);
     }
 }
 
@@ -573,7 +624,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         if (it != mapTx.end()) {
             setEntries stage;
             stage.insert(it);
-            RemoveStaged(stage);
+            RemoveStaged(stage, true);
         }
         removeConflicts(*tx, conflicts);
         ClearPrioritisation(tx->GetHash());
@@ -632,6 +683,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
         innerUsage += memusage::DynamicUsage(links.parents) + memusage::DynamicUsage(links.children);
         bool fDependsWait = false;
         setEntries setParentCheck;
+        int64_t parentSizes = 0;
+        unsigned int parentSigOpCount = 0;
         bool fHasZerocoinSpends = false;
         for (const CTxIn& txin : tx.vin) {
             // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's.
@@ -640,7 +693,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
                 const CTransaction& tx2 = it2->GetTx();
                 assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull());
                 fDependsWait = true;
-                setParentCheck.insert(it2);
+                if (setParentCheck.insert(it2).second) {
+                    parentSizes += it2->GetTxSize();
+                    parentSigOpCount += it2->GetSigOpCount();
+                }
             } else if(!txin.IsZerocoinSpend() && !txin.IsZerocoinPublicSpend()) {
                 assert(pcoins->HaveCoin(txin.prevout));
             } else {
@@ -666,18 +722,20 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             }
         }
         assert(setParentCheck == GetMemPoolParents(it));
+        // Also check to make sure ancestor size/sigops are >= sum with immediate
+        // parents.
+        assert(it->GetSizeWithAncestors() >= parentSizes + it->GetTxSize());
+        assert(it->GetSigOpCountWithAncestors() >= parentSigOpCount + it->GetSigOpCount());
         // Check children against mapNextTx
         if (!fHasZerocoinSpends) {
             CTxMemPool::setEntries setChildrenCheck;
             std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(tx.GetHash(), 0));
             int64_t childSizes = 0;
-            CAmount childModFee = 0;
             for (; iter != mapNextTx.end() && iter->first.hash == tx.GetHash(); ++iter) {
                 txiter childit = mapTx.find(iter->second.ptx->GetHash());
                 assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
                 if (setChildrenCheck.insert(childit).second) {
                     childSizes += childit->GetTxSize();
-                    childModFee += childit->GetModifiedFee();
                 }
             }
             assert(setChildrenCheck == GetMemPoolChildren(it));
@@ -919,10 +977,10 @@ size_t CTxMemPool::DynamicMemoryUsage() const
             memusage::DynamicUsage(mapSaplingNullifiers);
 }
 
-void CTxMemPool::RemoveStaged(setEntries &stage)
+void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants)
 {
     AssertLockHeld(cs);
-    UpdateForRemoveFromMempool(stage);
+    UpdateForRemoveFromMempool(stage, updateDescendants);
     for (const txiter& it : stage) {
         removeUnchecked(it);
     }
@@ -941,7 +999,7 @@ int CTxMemPool::Expire(int64_t time)
     for (const txiter& removeit : toremove) {
         CalculateDescendants(removeit, stage);
     }
-    RemoveStaged(stage);
+    RemoveStaged(stage, false);
     return stage.size();
 }
 
@@ -1052,7 +1110,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
             for (txiter it: stage)
                 txn.push_back(it->GetTx());
         }
-        RemoveStaged(stage);
+        RemoveStaged(stage, false);
         if (pvNoSpendsRemaining) {
             for (const CTransaction& tx: txn) {
                 for (const CTxIn& txin: tx.vin) {
diff --git a/src/txmempool.h b/src/txmempool.h
index 79b9773526e8..fa9682be70f1 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -87,6 +87,12 @@ class CTxMemPoolEntry
     uint64_t nSizeWithDescendants;  //! ... and size
     CAmount nModFeesWithDescendants;  //! ... and total fees (all including us)
 
+    // Analogous statistics for ancestor transactions
+    uint64_t nCountWithAncestors;
+    uint64_t nSizeWithAncestors;
+    CAmount nModFeesWithAncestors;
+    unsigned int nSigOpCountWithAncestors;
+
 public:
     CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee,
             int64_t _nTime, double _entryPriority, unsigned int _entryHeight,
@@ -113,7 +119,9 @@ class CTxMemPoolEntry
     size_t DynamicMemoryUsage() const { return nUsageSize; }
 
     // Adjusts the descendant state, if this entry is not dirty.
-    void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount);
+    void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount);
+    // Adjusts the ancestor state
+    void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps);
     // Updates the fee delta used for mining priority score, and the
     // modified fees with descendants.
     void UpdateFeeDelta(int64_t feeDelta);
@@ -123,6 +131,11 @@ class CTxMemPoolEntry
     CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
 
     bool GetSpendsCoinbaseOrCoinstake() const { return spendsCoinbaseOrCoinstake; }
+
+    uint64_t GetCountWithAncestors() const { return nCountWithAncestors; }
+    uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
+    CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
+    unsigned int GetSigOpCountWithAncestors() const { return nSigOpCountWithAncestors; }
 };
 
 // Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index.
@@ -133,7 +146,7 @@ struct update_descendant_state
     {}
 
     void operator() (CTxMemPoolEntry &e)
-        { e.UpdateState(modifySize, modifyFee, modifyCount); }
+        { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); }
 
     private:
         int64_t modifySize;
@@ -141,6 +154,22 @@ struct update_descendant_state
         int64_t modifyCount;
 };
 
+struct update_ancestor_state
+{
+    update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int _modifySigOps) :
+            modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOps(_modifySigOps)
+    {}
+
+    void operator() (CTxMemPoolEntry &e)
+    { e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOps); }
+
+private:
+    int64_t modifySize;
+    CAmount modifyFee;
+    int64_t modifyCount;
+    int modifySigOps;
+};
+
 struct update_fee_delta
 {
     update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { }
@@ -492,8 +521,12 @@ class CTxMemPool
 
     /** Remove a set of transactions from the mempool.
      *  If a transaction is in this set, then all in-mempool descendants must
-     *  also be in the set.*/
-    void RemoveStaged(setEntries &stage);
+     *  also be in the set, unless this transaction is being removed for being
+     *  in a block.
+     *  Set updateDescendants to true when removing a tx that was in a block, so
+     *  that any in-mempool descendants have their ancestor state updated.
+     */
+    void RemoveStaged(setEntries &stage, bool updateDescendants);
 
     /** When adding transactions from a disconnected block back to the mempool,
      *  new mempool entries may have children in the mempool (which is generally
@@ -604,8 +637,12 @@ class CTxMemPool
             const std::set<uint256> &setExclude);
     /** Update ancestors of hash to add/remove it as a descendant transaction. */
     void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors);
-    /** For each transaction being removed, update ancestors and any direct children. */
-    void UpdateForRemoveFromMempool(const setEntries &entriesToRemove);
+    /** Set ancestor state for an entry */
+    void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors);
+    /** For each transaction being removed, update ancestors and any direct children.
+      * If updateDescendants is true, then also update in-mempool descendants'
+      * ancestor state. */
+    void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants);
     /** Sever link between specified transaction and direct children. */
     void UpdateChildrenForRemoval(txiter entry);
     /** Populate setDescendants with all in-mempool descendants of hash.
diff --git a/src/validation.cpp b/src/validation.cpp
index d2feed1c0b04..4da8cb15c5f4 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -589,6 +589,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
             return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s",
                     __func__, hash.ToString(), FormatStateMessage(state));
         }
+        // todo: pool.removeStaged for all conflicting entries
 
         // Store transaction in memory
         pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload());

From 91c60962580b475977794a216e6f1b5169310be9 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Mon, 19 Oct 2015 15:15:12 -0400
Subject: [PATCH 06/32] Add ancestor feerate index to mempool

---
 src/test/mempool_tests.cpp | 104 +++++++++++++++++++++++++++++++++++++
 src/txmempool.cpp          |   4 +-
 src/txmempool.h            |  30 +++++++++++
 3 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 7d494f844f2f..18fd9088d7ef 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -316,6 +316,110 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     CheckSort<mining_score>(pool, sortedOrder);
 }
 
+BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
+{
+    CTxMemPool pool(CFeeRate(0));
+    TestMemPoolEntryHelper entry;
+    entry.hadNoDependencies = true;
+
+    /* 3rd highest fee */
+    CMutableTransaction tx1 = CMutableTransaction();
+    tx1.vout.resize(1);
+    tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx1.vout[0].nValue = 10 * COIN;
+    pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).Priority(10.0).FromTx(tx1));
+
+    /* highest fee */
+    CMutableTransaction tx2 = CMutableTransaction();
+    tx2.vout.resize(1);
+    tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx2.vout[0].nValue = 2 * COIN;
+    pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).Priority(9.0).FromTx(tx2));
+    uint64_t tx2Size = ::GetSerializeSize(tx2, SER_NETWORK, PROTOCOL_VERSION);
+
+    /* lowest fee */
+    CMutableTransaction tx3 = CMutableTransaction();
+    tx3.vout.resize(1);
+    tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx3.vout[0].nValue = 5 * COIN;
+    pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).Priority(100.0).FromTx(tx3));
+
+    /* 2nd highest fee */
+    CMutableTransaction tx4 = CMutableTransaction();
+    tx4.vout.resize(1);
+    tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx4.vout[0].nValue = 6 * COIN;
+    pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).Priority(1.0).FromTx(tx4));
+
+    /* equal fee rate to tx1, but newer */
+    CMutableTransaction tx5 = CMutableTransaction();
+    tx5.vout.resize(1);
+    tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx5.vout[0].nValue = 11 * COIN;
+    pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5));
+    BOOST_CHECK_EQUAL(pool.size(), 5);
+
+    std::vector<std::string> sortedOrder;
+    sortedOrder.resize(5);
+    sortedOrder[0] = tx2.GetHash().ToString(); // 20000
+    sortedOrder[1] = tx4.GetHash().ToString(); // 15000
+    // tx1 and tx5 are both 10000
+    // Ties are broken by hash, not timestamp, so determine which
+    // hash comes first.
+    if (tx1.GetHash() < tx5.GetHash()) {
+        sortedOrder[2] = tx1.GetHash().ToString();
+        sortedOrder[3] = tx5.GetHash().ToString();
+    } else {
+        sortedOrder[2] = tx5.GetHash().ToString();
+        sortedOrder[3] = tx1.GetHash().ToString();
+    }
+    sortedOrder[4] = tx3.GetHash().ToString(); // 0
+
+    CheckSort<ancestor_score>(pool, sortedOrder);
+
+    /* low fee parent with high fee child */
+    /* tx6 (0) -> tx7 (high) */
+    CMutableTransaction tx6 = CMutableTransaction();
+    tx6.vout.resize(1);
+    tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx6.vout[0].nValue = 20 * COIN;
+    uint64_t tx6Size = ::GetSerializeSize(tx6, SER_NETWORK, PROTOCOL_VERSION);
+
+    pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6));
+    BOOST_CHECK_EQUAL(pool.size(), 6);
+    sortedOrder.push_back(tx6.GetHash().ToString());
+    CheckSort<ancestor_score>(pool, sortedOrder);
+
+    CMutableTransaction tx7 = CMutableTransaction();
+    tx7.vin.resize(1);
+    tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0);
+    tx7.vin[0].scriptSig = CScript() << OP_11;
+    tx7.vout.resize(1);
+    tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
+    tx7.vout[0].nValue = 10 * COIN;
+    uint64_t tx7Size = ::GetSerializeSize(tx7, SER_NETWORK, PROTOCOL_VERSION);
+
+    /* set the fee to just below tx2's feerate when including ancestor */
+    CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1;
+
+    //CTxMemPoolEntry entry7(tx7, fee, 2, 10.0, 1, true);
+    pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7));
+    BOOST_CHECK_EQUAL(pool.size(), 7);
+    sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString());
+    CheckSort<ancestor_score>(pool, sortedOrder);
+
+    /* after tx6 is mined, tx7 should move up in the sort */
+    std::vector<CTransactionRef> vtx;
+    vtx.emplace_back(MakeTransactionRef(tx6));
+    std::list<CTransactionRef> dummy;
+    pool.removeForBlock(vtx, 1, dummy, false);
+
+    sortedOrder.erase(sortedOrder.begin()+1);
+    sortedOrder.pop_back();
+    sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString());
+    CheckSort<ancestor_score>(pool, sortedOrder);
+}
+
 
 BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
 {
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 1b50ee4b7dc4..a46774027479 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -967,9 +967,9 @@ bool CCoinsViewMemPool::GetNullifier(const uint256& nullifier) const
 size_t CTxMemPool::DynamicMemoryUsage() const
 {
     LOCK(cs);
-    // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for
+    // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for
     // boost::multi_index_contained is implemented.
-    return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 12 * sizeof(void*)) * mapTx.size() +
+    return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() +
             memusage::DynamicUsage(mapNextTx) +
             memusage::DynamicUsage(mapDeltas) +
             memusage::DynamicUsage(mapLinks) +
diff --git a/src/txmempool.h b/src/txmempool.h
index fa9682be70f1..05782badefbf 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -254,10 +254,34 @@ class CompareTxMemPoolEntryByEntryTime
     }
 };
 
+class CompareTxMemPoolEntryByAncestorFee
+{
+public:
+    bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
+    {
+        double aFees = a.GetModFeesWithAncestors();
+        double aSize = a.GetSizeWithAncestors();
+
+        double bFees = b.GetModFeesWithAncestors();
+        double bSize = b.GetSizeWithAncestors();
+
+        // Avoid division by rewriting (a/b > c/d) as (a*d > c*b).
+        double f1 = aFees * bSize;
+        double f2 = aSize * bFees;
+
+        if (f1 == f2) {
+            return a.GetTx().GetHash() < b.GetTx().GetHash();
+        }
+
+        return f1 > f2;
+    }
+};
+
 // Multi_index tag names
 struct descendant_score {};
 struct entry_time {};
 struct mining_score {};
+struct ancestor_score {};
 
 class CBlockPolicyEstimator;
 
@@ -410,6 +434,12 @@ class CTxMemPool
                 boost::multi_index::tag<mining_score>,
                 boost::multi_index::identity<CTxMemPoolEntry>,
                 CompareTxMemPoolEntryByScore
+            >,
+            // sorted by fee rate with ancestors
+            boost::multi_index::ordered_non_unique<
+                boost::multi_index::tag<ancestor_score>,
+                boost::multi_index::identity<CTxMemPoolEntry>,
+                CompareTxMemPoolEntryByAncestorFee
             >
         >
     > indexed_transaction_set;

From 8c0016e27cf1c1e8f22a1d5c19086f46b8cf071d Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 24 Dec 2020 19:45:18 -0300
Subject: [PATCH 07/32] Check all ancestor state in CTxMemPool::check()

Coming from btc@ce019bf90fe89c1256a89c489795987ef0b8a18f
---
 src/txmempool.cpp | 27 ++++++++++++++++++++++-----
 src/txmempool.h   |  2 +-
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index a46774027479..bc966a3872f9 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -160,7 +160,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
     }
 }
 
-bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */)
+bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const
 {
     setEntries parentHashes;
     const auto &tx = entry.GetSharedTx();
@@ -722,10 +722,27 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             }
         }
         assert(setParentCheck == GetMemPoolParents(it));
-        // Also check to make sure ancestor size/sigops are >= sum with immediate
-        // parents.
-        assert(it->GetSizeWithAncestors() >= parentSizes + it->GetTxSize());
-        assert(it->GetSigOpCountWithAncestors() >= parentSigOpCount + it->GetSigOpCount());
+        // Verify ancestor state is correct.
+        setEntries setAncestors;
+        uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();
+        std::string dummy;
+        CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy);
+        uint64_t nCountCheck = setAncestors.size() + 1;
+        uint64_t nSizeCheck = it->GetTxSize();
+        CAmount nFeesCheck = it->GetModifiedFee();
+        unsigned int nSigOpCheck = it->GetSigOpCount();
+
+        for (const txiter& ancestorIt : setAncestors) {
+            nSizeCheck += ancestorIt->GetTxSize();
+            nFeesCheck += ancestorIt->GetModifiedFee();
+            nSigOpCheck += ancestorIt->GetSigOpCount();
+        }
+
+        assert(it->GetCountWithAncestors() == nCountCheck);
+        assert(it->GetSizeWithAncestors() == nSizeCheck);
+        assert(it->GetSigOpCountWithAncestors() == nSigOpCheck);
+        assert(it->GetModFeesWithAncestors() == nFeesCheck);
+
         // Check children against mapNextTx
         if (!fHasZerocoinSpends) {
             CTxMemPool::setEntries setChildrenCheck;
diff --git a/src/txmempool.h b/src/txmempool.h
index 05782badefbf..9c9d5cbcf96b 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -579,7 +579,7 @@ class CTxMemPool
      *  fSearchForParents = whether to search a tx's vin for in-mempool parents, or
      *    look up parents from mapLinks. Must be true for entries not in the mempool
      */
-    bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true);
+    bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true) const;
 
     /** The minimum fee to get into the mempool, which may itself not be enough
      *  for larger-sized transactions.

From 6ebfd17548500df4b74cb75420839b345ea8af50 Mon Sep 17 00:00:00 2001
From: Alex Morcos <morcos@chaincode.com>
Date: Tue, 6 Dec 2016 16:49:32 -0500
Subject: [PATCH 08/32] tiny test fix for mempool_tests

---
 src/test/mempool_tests.cpp | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 18fd9088d7ef..b4a545a182b9 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -387,7 +387,12 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
 
     pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6));
     BOOST_CHECK_EQUAL(pool.size(), 6);
-    sortedOrder.push_back(tx6.GetHash().ToString());
+    // Ties are broken by hash
+    if (tx3.GetHash() < tx6.GetHash())
+        sortedOrder.push_back(tx6.GetHash().ToString());
+    else
+        sortedOrder.insert(sortedOrder.end()-1,tx6.GetHash().ToString());
+
     CheckSort<ancestor_score>(pool, sortedOrder);
 
     CMutableTransaction tx7 = CMutableTransaction();
@@ -415,7 +420,11 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
     pool.removeForBlock(vtx, 1, dummy, false);
 
     sortedOrder.erase(sortedOrder.begin()+1);
-    sortedOrder.pop_back();
+    // Ties are broken by hash
+    if (tx3.GetHash() < tx6.GetHash())
+        sortedOrder.pop_back();
+    else
+        sortedOrder.erase(sortedOrder.end()-2);
     sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString());
     CheckSort<ancestor_score>(pool, sortedOrder);
 }

From 23c9f3e85fc32d24f724bf309727187e74c2519f Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Dec 2020 21:16:23 -0300
Subject: [PATCH 09/32] Eliminate TX trickle bypass, sort TX invs for privacy
 and priority.

Previously Bitcoin would send 1/4 of transactions out to all peers
instantly. This causes high overhead because it makes >80% of
INVs size 1. Doing so harms privacy, because it limits the
amount of source obscurity a transaction can receive.

These randomized broadcasts also disobeyed transaction dependencies
and required use of the orphan pool. Because the orphan pool is
so small this leads to poor propagation for dependent transactions.

When the bypass wasn't in effect, transactions were sent in the
order they were received. This avoided creating orphans but
undermines privacy fairly significantly.

This commit:
Eliminates the bypass. The bypass is replaced by halving the
 average delay for outbound peers.

Sorts candidate transactions for INV by their topological
 depth then by their feerate (then hash); removing the
 information leakage and providing priority service to
 higher fee transactions.

Limits the amount of transactions sent in a single INV to
 7tx/sec (and twice that for outbound); this limits the
 harm of low fee transaction floods, gives faster relay
 service to higher fee transactions. The 7 sounds lower
 than it really is because received advertisements need
 not be sent, and because the aggregate rate is multipled
 by the number of peers.

 Coming from btc@f2d3ba73860e875972738d1da1507124d0971ae5
---
 src/net_processing.cpp | 56 +++++++++++++++++++++++++++---------------
 src/net_processing.h   |  7 ++++++
 src/txmempool.cpp      | 15 +++++++++++
 src/txmempool.h        |  1 +
 src/validation.h       |  3 ---
 5 files changed, 59 insertions(+), 23 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 48de89d900f8..39309645908e 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1940,6 +1940,29 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic<bool>& interru
     return fMoreWork;
 }
 
+class CompareInvMempoolOrder
+{
+    CTxMemPool *mp;
+public:
+    CompareInvMempoolOrder(CTxMemPool *mempool)
+    {
+        mp = mempool;
+    }
+
+    bool operator()(const CInv &a, const CInv &b)
+    {
+        if (a.type != MSG_TX && b.type != MSG_TX) {
+            return false;
+        } else {
+            if (a.type != MSG_TX) {
+                return true;
+            } else if (b.type != MSG_TX) {
+                return false;
+            }
+            return mp->CompareDepthAndScore(a.hash, b.hash);
+        }
+    }
+};
 
 bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsgProc)
 {
@@ -2067,38 +2090,31 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
             bool fSendTrickle = pto->fWhitelisted;
             if (pto->nNextInvSend < nNow) {
                 fSendTrickle = true;
-                pto->nNextInvSend = PoissonNextSend(nNow, AVG_INVENTORY_BROADCAST_INTERVAL);
+                // Use half the delay for outbound peers, as their is less privacy concern for them.
+                pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound);
             }
             LOCK(pto->cs_inventory);
-            vInv.reserve(pto->vInventoryToSend.size());
+            if (fSendTrickle && pto->vInventoryToSend.size() > 1) {
+                // Topologically and fee-rate sort the inventory we send for privacy and priority reasons.
+                CompareInvMempoolOrder compareInvMempoolOrder(&mempool);
+                std::stable_sort(pto->vInventoryToSend.begin(), pto->vInventoryToSend.end(), compareInvMempoolOrder);
+            }
+            vInv.reserve(std::min<size_t>(INVENTORY_BROADCAST_MAX, pto->vInventoryToSend.size()));
             vInvWait.reserve(pto->vInventoryToSend.size());
             for (const CInv& inv : pto->vInventoryToSend) {
                 if (inv.type == MSG_TX && pto->filterInventoryKnown.contains(inv.hash))
                     continue;
 
-                // trickle out tx inv to protect privacy
-                if (inv.type == MSG_TX && !fSendTrickle) {
-                    // 1/4 of tx invs blast to all immediately
-                    static uint256 hashSalt;
-                    if (hashSalt.IsNull())
-                        hashSalt = GetRandHash();
-                    uint256 hashRand = inv.hash ^ hashSalt;
-                    hashRand = Hash(BEGIN(hashRand), END(hashRand));
-                    bool fTrickleWait = ((hashRand & 3) != 0);
-
-                    if (fTrickleWait) {
-                        vInvWait.push_back(inv);
-                        continue;
-                    }
+                // No reason to drain out at many times the network's capacity,
+                // especially since we have many peers and some will draw much shorter delays.
+                if (vInv.size() >= INVENTORY_BROADCAST_MAX || (inv.type == MSG_TX && !fSendTrickle)) {
+                    vInvWait.push_back(inv);
+                    continue;
                 }
 
                 pto->filterInventoryKnown.insert(inv.hash);
 
                 vInv.push_back(inv);
-                if (vInv.size() >= 1000) {
-                    connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
-                    vInv.clear();
-                }
             }
             pto->vInventoryToSend = vInvWait;
         }
diff --git a/src/net_processing.h b/src/net_processing.h
index c524e59ed751..9924cf704d00 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -21,6 +21,13 @@ static const unsigned int DEFAULT_BLOCK_SPAM_FILTER_MAX_SIZE = 100;
 /** Default for -blockspamfiltermaxavg, maximum average size of an index occurrence in the block spam filter */
 static const unsigned int DEFAULT_BLOCK_SPAM_FILTER_MAX_AVG = 10;
 
+/** Average delay between trickled inventory transmissions in seconds.
+ *  Blocks and whitelisted receivers bypass this, outbound peers get half this delay. */
+static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5;
+/** Maximum number of inventory items to send per transmission.
+ *  Limits the impact of low-fee transaction floods. */
+static const unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL;
+
 /** Register with a network node to receive its signals */
 void RegisterNodeSignals(CNodeSignals& nodeSignals);
 /** Unregister a network node */
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index bc966a3872f9..23cfa1bdc9de 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -815,6 +815,21 @@ void CTxMemPool::checkNullifiers() const
     }
 }
 
+bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb)
+{
+    LOCK(cs);
+    indexed_transaction_set::const_iterator i = mapTx.find(hasha);
+    if (i == mapTx.end()) return false;
+    indexed_transaction_set::const_iterator j = mapTx.find(hashb);
+    if (j == mapTx.end()) return true;
+    uint64_t counta = i->GetCountWithAncestors();
+    uint64_t countb = j->GetCountWithAncestors();
+    if (counta == countb) {
+        return CompareTxMemPoolEntryByScore()(*i, *j);
+    }
+    return counta < countb;
+}
+
 void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
 {
     vtxid.clear();
diff --git a/src/txmempool.h b/src/txmempool.h
index 9c9d5cbcf96b..73cf2ef5a125 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -531,6 +531,7 @@ class CTxMemPool
     void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight, std::list<CTransactionRef>& conflicts, bool fCurrentEstimate = true);
     void clear();
     void _clear();  // lock-free
+    bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);
     void queryHashes(std::vector<uint256>& vtxid);
     void getTransactions(std::set<uint256>& setTxid);
     bool isSpent(const COutPoint& outpoint);
diff --git a/src/validation.h b/src/validation.h
index 98965ee455f0..fa4c987ce318 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -107,9 +107,6 @@ static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111;
 static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60;
 /** Average delay between peer address broadcasts in seconds. */
 static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30;
-/** Average delay between trickled inventory broadcasts in seconds.
- *  Blocks, whitelisted receivers, and a random 25% of transactions bypass this. */
-static const unsigned int AVG_INVENTORY_BROADCAST_INTERVAL = 5;
 /** Default multiplier used in the computation for shielded txes min fee */
 static const unsigned int DEFAULT_SHIELDEDTXFEE_K = 100;
 

From 191c62ea0cce427e83f36bb4b0875b3d7ac9f2c9 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter.wuille@gmail.com>
Date: Sun, 10 Apr 2016 15:33:05 +0200
Subject: [PATCH 10/32] Return mempool queries in dependency order

---
 src/txmempool.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 23cfa1bdc9de..da62d5cc738c 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -830,6 +830,16 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb
     return counta < countb;
 }
 
+namespace {
+class DepthAndScoreComparator
+{
+    CTxMemPool *mp;
+public:
+    DepthAndScoreComparator(CTxMemPool *mempool) : mp(mempool) {}
+    bool operator()(const uint256& a, const uint256& b) { return mp->CompareDepthAndScore(a, b); }
+};
+}
+
 void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
 {
     vtxid.clear();
@@ -838,6 +848,8 @@ void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
     vtxid.reserve(mapTx.size());
     for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi)
         vtxid.push_back(mi->GetTx().GetHash());
+
+    std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this));
 }
 
 void CTxMemPool::getTransactions(std::set<uint256>& setTxid)

From 76248235435d37474fb9eb17d6f897912b860a50 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Fri, 25 Dec 2020 23:25:10 -0300
Subject: [PATCH 11/32] mapNextTx: use pointer as key, simplify value

Saves about 10% of application memory usage once the mempool warms up. Since the
mempool is DynamicUsage-regulated, this will translate to a larger mempool in
the same amount of space.

Map value type: eliminate the vin index; no users of the map need to know which
input of the transaction is spending the prevout.

Map key type: replace the COutPoint with a pointer to a COutPoint. A COutPoint
is 36 bytes, but each COutPoint is accessible from the same map entry's value.
A trivial DereferencingComparator functor allows indirect map keys, but the
resulting syntax is misleading: `map.find(&outpoint)`. Implement an indirectmap
that acts as a wrapper to a map that uses a DereferencingComparator, supporting
a syntax that accurately reflect the container's semantics: inserts and
iterators use pointers since they store pointers and need them to remain
constant and dereferenceable, but lookup functions take const references.

 Coming from btc@9805f4af7ecb6becf8a146bd845fb131ffa625c9
---
 CMakeLists.txt    |  1 +
 src/Makefile.am   |  1 +
 src/indirectmap.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 src/memusage.h    | 15 ++++++++++++
 src/txmempool.cpp | 38 +++++++++++++++----------------
 src/txmempool.h   | 25 ++------------------
 6 files changed, 95 insertions(+), 43 deletions(-)
 create mode 100644 src/indirectmap.h

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 931aebf4b275..6a88776965af 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -193,6 +193,7 @@ set(SERVER_SOURCES
         ./src/checkpoints.cpp
         ./src/httprpc.cpp
         ./src/httpserver.cpp
+        ./src/indirectmap.h
         ./src/init.cpp
         ./src/interfaces/handler.cpp
         ./src/interfaces/wallet.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 986fd6cd8a68..f2c2a834e9ca 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -189,6 +189,7 @@ BITCOIN_CORE_H = \
   hash.h \
   httprpc.h \
   httpserver.h \
+  indirectmap.h \
   init.h \
   interfaces/handler.h \
   interfaces/wallet.h \
diff --git a/src/indirectmap.h b/src/indirectmap.h
new file mode 100644
index 000000000000..af8f23b46fc4
--- /dev/null
+++ b/src/indirectmap.h
@@ -0,0 +1,58 @@
+// Copyright (c) 2016-2020 The Bitcoin developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_INDIRECTMAP_H
+#define BITCOIN_INDIRECTMAP_H
+
+#include <map>
+
+template <class T>
+struct DereferencingComparator { bool operator()(const T a, const T b) const { return *a < *b; } };
+
+/* Map whose keys are pointers, but are compared by their dereferenced values.
+ *
+ * Differs from a plain std::map<const K*, T, DereferencingComparator<K*> > in
+ * that methods that take a key for comparison take a K rather than taking a K*
+ * (taking a K* would be confusing, since it's the value rather than the address
+ * of the object for comparison that matters due to the dereferencing comparator).
+ *
+ * Objects pointed to by keys must not be modified in any way that changes the
+ * result of DereferencingComparator.
+ */
+template <class K, class T>
+class indirectmap {
+private:
+    typedef std::map<const K*, T, DereferencingComparator<const K*> > base;
+    base m;
+public:
+    typedef typename base::iterator iterator;
+    typedef typename base::const_iterator const_iterator;
+    typedef typename base::size_type size_type;
+    typedef typename base::value_type value_type;
+
+    // passthrough (pointer interface)
+    std::pair<iterator, bool> insert(const value_type& value) { return m.insert(value); }
+
+    // pass address (value interface)
+    iterator find(const K& key)                     { return m.find(&key); }
+    const_iterator find(const K& key) const         { return m.find(&key); }
+    iterator lower_bound(const K& key)              { return m.lower_bound(&key); }
+    const_iterator lower_bound(const K& key) const  { return m.lower_bound(&key); }
+    size_type erase(const K& key)                   { return m.erase(&key); }
+    size_type count(const K& key) const             { return m.count(&key); }
+
+    // passthrough
+    bool empty() const              { return m.empty(); }
+    size_type size() const          { return m.size(); }
+    size_type max_size() const      { return m.max_size(); }
+    void clear()                    { m.clear(); }
+    iterator begin()                { return m.begin(); }
+    iterator end()                  { return m.end(); }
+    const_iterator begin() const    { return m.begin(); }
+    const_iterator end() const      { return m.end(); }
+    const_iterator cbegin() const   { return m.cbegin(); }
+    const_iterator cend() const     { return m.cend(); }
+};
+
+#endif // BITCOIN_INDIRECTMAP_H
diff --git a/src/memusage.h b/src/memusage.h
index b06421a75017..f2a56b6b9b90 100644
--- a/src/memusage.h
+++ b/src/memusage.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_MEMUSAGE_H
 #define BITCOIN_MEMUSAGE_H
 
+#include "indirectmap.h"
 #include "prevector.h"
 
 #include <stdlib.h>
@@ -185,6 +186,20 @@ static inline size_t DynamicUsage(const std::shared_ptr<X>& p)
     return p ? MallocUsage(sizeof(X)) + MallocUsage(sizeof(stl_shared_counter)) : 0;
 }
 
+// indirectmap has underlying map with pointer as key
+
+template<typename X, typename Y>
+static inline size_t DynamicUsage(const indirectmap<X, Y>& m)
+{
+    return MallocUsage(sizeof(stl_tree_node<std::pair<const X*, Y> >)) * m.size();
+}
+
+template<typename X, typename Y>
+static inline size_t IncrementalDynamicUsage(const indirectmap<X, Y>& m)
+{
+    return MallocUsage(sizeof(stl_tree_node<std::pair<const X*, Y> >));
+}
+
 // Boost data structures
 
 template<typename X>
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index da62d5cc738c..cfda1f746c5a 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -142,11 +142,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
         if (it == mapTx.end()) {
             continue;
         }
-        std::map<COutPoint, CInPoint>::iterator iter = mapNextTx.lower_bound(COutPoint(hash, 0));
+        auto iter = mapNextTx.lower_bound(COutPoint(hash, 0));
         // First calculate the children, and update setMemPoolChildren to
         // include them, and update their setMemPoolParents to include this tx.
-        for (; iter != mapNextTx.end() && iter->first.hash == hash; ++iter) {
-            const uint256 &childHash = iter->second.ptx->GetHash();
+        for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) {
+            const uint256 &childHash = iter->second->GetHash();
             txiter childIter = mapTx.find(childHash);
             assert(childIter != mapTx.end());
             // We can skip updating entries we've encountered before or that
@@ -404,7 +404,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
     std::set<uint256> setParentTransactions;
     if(!tx.HasZerocoinSpendInputs()) {
         for (unsigned int i = 0; i < tx.vin.size(); i++) {
-            mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i);
+            mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit->GetSharedTx()));
             setParentTransactions.insert(tx.vin[i].prevout.hash);
         }
     }
@@ -504,10 +504,10 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransact
             // happen during chain re-orgs if origTx isn't re-accepted into
             // the mempool for any reason.
             for (unsigned int i = 0; i < origTx.vout.size(); i++) {
-                std::map<COutPoint, CInPoint>::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i));
+                auto it = mapNextTx.find(COutPoint(origTx.GetHash(), i));
                 if (it == mapNextTx.end())
                     continue;
-                txiter nextit = mapTx.find(it->second.ptx->GetHash());
+                txiter nextit = mapTx.find(it->second->GetHash());
                 assert(nextit != mapTx.end());
                 txToRemove.insert(nextit);
             }
@@ -584,9 +584,9 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
     std::list<CTransaction> result;
     LOCK(cs);
     for (const CTxIn& txin : tx.vin) {
-        std::map<COutPoint, CInPoint>::iterator it = mapNextTx.find(txin.prevout);
+        auto it = mapNextTx.find(txin.prevout);
         if (it != mapNextTx.end()) {
-            const CTransaction& txConflict = *it->second.ptx;
+            const CTransaction& txConflict = *it->second;
             if (txConflict != tx) {
                 removeRecursive(txConflict, removed);
             }
@@ -704,10 +704,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             }
             // Check whether its inputs are marked in mapNextTx.
             if(!fHasZerocoinSpends) {
-                std::map<COutPoint, CInPoint>::const_iterator it3 = mapNextTx.find(txin.prevout);
+                auto it3 = mapNextTx.find(txin.prevout);
                 assert(it3 != mapNextTx.end());
-                assert(it3->second.ptx == &tx);
-                assert(it3->second.n == i);
+                assert(it3->first == &txin.prevout);
+                assert(*it3->second == tx);
             } else {
                 fDependsWait=false;
             }
@@ -746,10 +746,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
         // Check children against mapNextTx
         if (!fHasZerocoinSpends) {
             CTxMemPool::setEntries setChildrenCheck;
-            std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(tx.GetHash(), 0));
+            auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0));
             int64_t childSizes = 0;
-            for (; iter != mapNextTx.end() && iter->first.hash == tx.GetHash(); ++iter) {
-                txiter childit = mapTx.find(iter->second.ptx->GetHash());
+            for (; iter != mapNextTx.end() && iter->first->hash == tx.GetHash(); ++iter) {
+                txiter childit = mapTx.find(iter->second->GetHash());
                 assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
                 if (setChildrenCheck.insert(childit).second) {
                     childSizes += childit->GetTxSize();
@@ -787,14 +787,12 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
             stepsSinceLastRemove = 0;
         }
     }
-    for (std::map<COutPoint, CInPoint>::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) {
-        const uint256& hash = it->second.ptx->GetHash();
+    for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) {
+        const uint256& hash = it->second->GetHash();
         indexed_transaction_set::const_iterator it2 = mapTx.find(hash);
-        const CTransaction& tx = it2->GetTx();
+        const CTransactionRef& tx = it2->GetSharedTx();
         assert(it2 != mapTx.end());
-        assert(&tx == it->second.ptx);
-        assert(tx.vin.size() > it->second.n);
-        assert(it->first == it->second.ptx->vin[it->second.n].prevout);
+        assert(tx == it->second);
     }
 
     // Consistency check for sapling nullifiers
diff --git a/src/txmempool.h b/src/txmempool.h
index 73cf2ef5a125..715720eac7f7 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -13,6 +13,7 @@
 
 #include "amount.h"
 #include "coins.h"
+#include "indirectmap.h"
 #include "policy/feerate.h"
 #include "primitives/transaction.h"
 #include "sync.h"
@@ -285,28 +286,6 @@ struct ancestor_score {};
 
 class CBlockPolicyEstimator;
 
-/** An inpoint - a combination of a transaction and an index n into its vin */
-class CInPoint
-{
-public:
-    const CTransaction* ptx;
-    uint32_t n;
-
-    CInPoint() { SetNull(); }
-    CInPoint(const CTransaction* ptxIn, uint32_t nIn)
-    {
-        ptx = ptxIn;
-        n = nIn;
-    }
-    void SetNull()
-    {
-        ptx = NULL;
-        n = (uint32_t)-1;
-    }
-    bool IsNull() const { return (ptx == NULL && n == (uint32_t)-1); }
-    size_t DynamicMemoryUsage() const { return 0; }
-};
-
 /**
  * CTxMemPool stores valid-according-to-the-current-best-chain
  * transactions that may be included in the next block.
@@ -498,7 +477,7 @@ class CTxMemPool
     void UpdateChild(txiter entry, txiter child, bool add);
 
 public:
-    std::map<COutPoint, CInPoint> mapNextTx;
+    indirectmap<COutPoint, CTransactionRef> mapNextTx;
     std::map<uint256, std::pair<double, CAmount> > mapDeltas;
 
     /** Create a new CTxMemPool.

From 68bc68fd8bdd8a618deea213220e8c0664127151 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Mon, 28 Dec 2020 20:35:36 -0300
Subject: [PATCH 12/32] Don't do mempool lookups for "mempool" command without
 a filter

 Coming from btc@96918a2f0990a8207d7631b8de73af8ae5d24aeb
---
 src/net_processing.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 39309645908e..73ccd5ca9e1b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1640,12 +1640,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         std::vector<CInv> vInv;
         for (uint256& hash : vtxid) {
             CInv inv(MSG_TX, hash);
-            CTransaction tx;
-            bool fInMemPool = mempool.lookup(hash, tx);
-            if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
-            if ((pfrom->pfilter && pfrom->pfilter->IsRelevantAndUpdate(tx)) ||
-                (!pfrom->pfilter))
-                vInv.push_back(inv);
+            if (pfrom->pfilter) {
+                CTransaction tx;
+                bool fInMemPool = mempool.lookup(hash, tx);
+                if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
+                if (!pfrom->pfilter->IsRelevantAndUpdate(tx)) continue;
+            }
+            vInv.emplace_back(inv);
             if (vInv.size() == MAX_INV_SZ) {
                 connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv));
                 vInv.clear();

From 96457751b67e804a79e3327392719fa444c38f3f Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 31 Dec 2020 01:29:56 -0300
Subject: [PATCH 13/32] Split up and optimize transaction and block inv queues

An adaptation of btc@dc13dcd2bec2613a1cd5e0395b09b449d176146f with a tier two INV vector.
---
 src/net.h              | 24 +++++++++---
 src/net_processing.cpp | 89 +++++++++++++++++++++++++++---------------
 2 files changed, 75 insertions(+), 38 deletions(-)

diff --git a/src/net.h b/src/net.h
index 93cc53b72e8c..eba22964d448 100644
--- a/src/net.h
+++ b/src/net.h
@@ -610,7 +610,15 @@ class CNode
 
     // inventory based relay
     CRollingBloomFilter filterInventoryKnown;
-    std::vector<CInv> vInventoryToSend;
+    // Set of transaction ids we still have to announce.
+    // They are sorted by the mempool before relay, so the order is not important.
+    std::set<uint256> setInventoryTxToSend;
+    // List of block ids we still have announce.
+    // There is no final sorting before sending, as they are always sent immediately
+    // and in the order requested.
+    std::vector<uint256> vInventoryBlockToSend;
+    // Set of tier two messages ids we still have to announce.
+    std::vector<CInv> vInventoryTierTwoToSend;
     RecursiveMutex cs_inventory;
     std::multimap<int64_t, CInv> mapAskFor;
     std::vector<uint256> vBlockRequested;
@@ -735,11 +743,15 @@ class CNode
 
     void PushInventory(const CInv& inv)
     {
-        {
-            LOCK(cs_inventory);
-            if (inv.type == MSG_TX && filterInventoryKnown.contains(inv.hash))
-                return;
-            vInventoryToSend.push_back(inv);
+        LOCK(cs_inventory);
+        if (inv.type == MSG_TX) {
+            if (!filterInventoryKnown.contains(inv.hash)) {
+                setInventoryTxToSend.insert(inv.hash);
+            }
+        } else if (inv.type == MSG_BLOCK) {
+            vInventoryBlockToSend.push_back(inv.hash);
+        } else {
+            vInventoryTierTwoToSend.emplace_back(inv);
         }
     }
 
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 73ccd5ca9e1b..f07cf9077ac6 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1945,23 +1945,16 @@ class CompareInvMempoolOrder
 {
     CTxMemPool *mp;
 public:
-    CompareInvMempoolOrder(CTxMemPool *mempool)
+    CompareInvMempoolOrder(CTxMemPool *_mempool)
     {
-        mp = mempool;
+        mp = _mempool;
     }
 
-    bool operator()(const CInv &a, const CInv &b)
+    bool operator()(std::set<uint256>::iterator a, std::set<uint256>::iterator b)
     {
-        if (a.type != MSG_TX && b.type != MSG_TX) {
-            return false;
-        } else {
-            if (a.type != MSG_TX) {
-                return true;
-            } else if (b.type != MSG_TX) {
-                return false;
-            }
-            return mp->CompareDepthAndScore(a.hash, b.hash);
-        }
+        /* As std::make_heap produces a max-heap, we want the entries with the
+         * fewest ancestors/highest fee to sort later. */
+        return mp->CompareDepthAndScore(*b, *a);
     }
 };
 
@@ -2088,36 +2081,68 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
         std::vector<CInv> vInv;
         std::vector<CInv> vInvWait;
         {
+            LOCK(pto->cs_inventory);
+            vInv.reserve(std::max<size_t>(pto->vInventoryBlockToSend.size(), INVENTORY_BROADCAST_MAX));
+
+            // Add blocks
+            for (const uint256& hash : pto->vInventoryBlockToSend) {
+                vInv.emplace_back(CInv(MSG_BLOCK, hash));
+                if (vInv.size() == MAX_INV_SZ) {
+                    connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
+                    vInv.clear();
+                }
+            }
+            pto->vInventoryBlockToSend.clear();
+
+            // Add tier two INVs
+            for (const CInv& tInv : pto->vInventoryTierTwoToSend) {
+                vInv.emplace_back(tInv);
+                if (vInv.size() == MAX_INV_SZ) {
+                    connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
+                    vInv.clear();
+                }
+            }
+            pto->vInventoryTierTwoToSend.clear();
+
+            // Determine transactions to relay
             bool fSendTrickle = pto->fWhitelisted;
             if (pto->nNextInvSend < nNow) {
                 fSendTrickle = true;
-                // Use half the delay for outbound peers, as their is less privacy concern for them.
+                // Use half the delay for outbound peers, as there is less privacy concern for them.
                 pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound);
             }
-            LOCK(pto->cs_inventory);
-            if (fSendTrickle && pto->vInventoryToSend.size() > 1) {
+            if (fSendTrickle) {
+                // Produce a vector with all candidates for sending
+                std::vector<std::set<uint256>::iterator> vInvTx;
+                vInvTx.reserve(pto->setInventoryTxToSend.size());
+                for (std::set<uint256>::iterator it = pto->setInventoryTxToSend.begin(); it != pto->setInventoryTxToSend.end(); it++) {
+                    vInvTx.push_back(it);
+                }
                 // Topologically and fee-rate sort the inventory we send for privacy and priority reasons.
+                // A heap is used so that not all items need sorting if only a few are being sent.
                 CompareInvMempoolOrder compareInvMempoolOrder(&mempool);
-                std::stable_sort(pto->vInventoryToSend.begin(), pto->vInventoryToSend.end(), compareInvMempoolOrder);
-            }
-            vInv.reserve(std::min<size_t>(INVENTORY_BROADCAST_MAX, pto->vInventoryToSend.size()));
-            vInvWait.reserve(pto->vInventoryToSend.size());
-            for (const CInv& inv : pto->vInventoryToSend) {
-                if (inv.type == MSG_TX && pto->filterInventoryKnown.contains(inv.hash))
-                    continue;
-
+                std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
                 // No reason to drain out at many times the network's capacity,
                 // especially since we have many peers and some will draw much shorter delays.
-                if (vInv.size() >= INVENTORY_BROADCAST_MAX || (inv.type == MSG_TX && !fSendTrickle)) {
-                    vInvWait.push_back(inv);
-                    continue;
+                unsigned int nRelayedTransactions = 0;
+                while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {
+                    // Fetch the top element from the heap
+                    std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
+                    std::set<uint256>::iterator it = vInvTx.back();
+                    vInvTx.pop_back();
+                    uint256 hash = *it;
+                    // Remove it from the to-be-sent set
+                    pto->setInventoryTxToSend.erase(it);
+                    // Check if not in the filter already
+                    if (pto->filterInventoryKnown.contains(hash)) {
+                        continue;
+                    }
+                    // Send
+                    vInv.emplace_back(CInv(MSG_TX, hash));
+                    nRelayedTransactions++;
+                    pto->filterInventoryKnown.insert(hash);
                 }
-
-                pto->filterInventoryKnown.insert(inv.hash);
-
-                vInv.push_back(inv);
             }
-            pto->vInventoryToSend = vInvWait;
         }
         if (!vInv.empty())
             connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));

From cb4fc6c10732dc1a409fc80be3e7bafcea6cccd6 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 31 Dec 2020 12:18:35 -0300
Subject: [PATCH 14/32] An adapted version of
 btc@ed7068302c7490e8061cb3a558a0f83a465beeea

Handle mempool requests in send loop, subject to trickle  By eliminating queued entries from the mempool response and responding only at trickle time, this makes the mempool no longer leak transaction arrival order information (as the mempool itself is also sorted)-- at least no more than relay itself leaks it.
---
 src/net.cpp            |  1 +
 src/net.h              |  2 ++
 src/net_processing.cpp | 58 +++++++++++++++++++++++++++---------------
 3 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index ff41506d9c30..1727074eb5e2 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2438,6 +2438,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     hashContinue = UINT256_ZERO;
     nStartingHeight = -1;
     filterInventoryKnown.reset();
+    fSendMempool = false;
     fGetAddr = false;
     nNextLocalAddrSend = 0;
     nNextAddrSend = 0;
diff --git a/src/net.h b/src/net.h
index eba22964d448..a3c7e483639c 100644
--- a/src/net.h
+++ b/src/net.h
@@ -623,6 +623,8 @@ class CNode
     std::multimap<int64_t, CInv> mapAskFor;
     std::vector<uint256> vBlockRequested;
     int64_t nNextInvSend;
+    // Used for BIP35 mempool sending, also protected by cs_inventory
+    bool fSendMempool;
 
     // Ping time measurement:
     // The pong reply we're expecting, or 0 if no pong expected.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index f07cf9077ac6..3319518c2521 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1633,27 +1633,10 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
 
 
     else if (strCommand == NetMsgType::MEMPOOL) {
-        LOCK2(cs_main, pfrom->cs_filter);
 
-        std::vector<uint256> vtxid;
-        mempool.queryHashes(vtxid);
-        std::vector<CInv> vInv;
-        for (uint256& hash : vtxid) {
-            CInv inv(MSG_TX, hash);
-            if (pfrom->pfilter) {
-                CTransaction tx;
-                bool fInMemPool = mempool.lookup(hash, tx);
-                if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
-                if (!pfrom->pfilter->IsRelevantAndUpdate(tx)) continue;
-            }
-            vInv.emplace_back(inv);
-            if (vInv.size() == MAX_INV_SZ) {
-                connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv));
-                vInv.clear();
-            }
-        }
-        if (vInv.size() > 0)
-            connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv));
+        // todo: limit mempool request with a bandwidth limit
+        LOCK(pfrom->cs_inventory);
+        pfrom->fSendMempool = true;
     }
 
 
@@ -2104,13 +2087,42 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
             }
             pto->vInventoryTierTwoToSend.clear();
 
-            // Determine transactions to relay
+            // Check whether periodic send should happen
             bool fSendTrickle = pto->fWhitelisted;
             if (pto->nNextInvSend < nNow) {
                 fSendTrickle = true;
                 // Use half the delay for outbound peers, as there is less privacy concern for them.
                 pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound);
             }
+
+            // Respond to BIP35 mempool requests
+            if (fSendTrickle && pto->fSendMempool) {
+                std::vector<uint256> vtxid;
+                mempool.queryHashes(vtxid);
+                pto->fSendMempool = false;
+                // future: back port fee filter rate
+                LOCK(pto->cs_filter);
+
+                for (const uint256& hash : vtxid) {
+                    CInv inv(MSG_TX, hash);
+                    pto->setInventoryTxToSend.erase(hash);
+                    // future: add fee filter check here..
+                    if (pto->pfilter) {
+                        CTransaction tx;
+                        bool fInMemPool = mempool.lookup(hash, tx);
+                        if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
+                        if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue;
+                    }
+                    pto->filterInventoryKnown.insert(hash);
+                    vInv.emplace_back(inv);
+                    if (vInv.size() == MAX_INV_SZ) {
+                        connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
+                        vInv.clear();
+                    }
+                }
+            }
+
+            // Determine transactions to relay
             if (fSendTrickle) {
                 // Produce a vector with all candidates for sending
                 std::vector<std::set<uint256>::iterator> vInvTx;
@@ -2140,6 +2152,10 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                     // Send
                     vInv.emplace_back(CInv(MSG_TX, hash));
                     nRelayedTransactions++;
+                    if (vInv.size() == MAX_INV_SZ) {
+                        connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv));
+                        vInv.clear();
+                    }
                     pto->filterInventoryKnown.insert(hash);
                 }
             }

From d10583bb55e6a9afa410db07c32a7c0715853503 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 31 Dec 2020 12:55:35 -0300
Subject: [PATCH 15/32] An adapted version of
 btc@b5599147533103efea896a1fc4ff51f2d3ad5808

Move bloom and feerate filtering to just prior to tx sending.

This will avoid sending more pointless INVs around updates, and
prevents using filter updates to timetag transactions.

Also adds locking for fRelayTxes.
---
 src/net.h              |  2 +-
 src/net_processing.cpp | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/net.h b/src/net.h
index a3c7e483639c..765fcf3a156f 100644
--- a/src/net.h
+++ b/src/net.h
@@ -580,7 +580,7 @@ class CNode
     // a) it allows us to not relay tx invs before receiving the peer's version message
     // b) the peer may tell us in their version message that we should not relay tx invs
     //    until they have initialized their bloom filter.
-    bool fRelayTxes;
+    bool fRelayTxes; //protected by cs_filter
     CSemaphoreGrant grantOutbound;
     RecursiveMutex cs_filter;
     CBloomFilter* pfilter;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 3319518c2521..48ccd01ec9eb 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1053,10 +1053,9 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
             vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH);
             cleanSubVer = SanitizeString(strSubVer);
         }
-        if (!vRecv.empty())
+        if (!vRecv.empty()) {
             vRecv >> nStartingHeight;
-        if (!vRecv.empty())
-            vRecv >> fRelay;
+        }
 
         // Disconnect if we connected to ourself
         if (pfrom->fInbound && !connman.CheckIncomingNonce(nNonce)) {
@@ -1084,9 +1083,14 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         }
         pfrom->nStartingHeight = nStartingHeight;
         pfrom->fClient = !(nServices & NODE_NETWORK);
+
         {
             LOCK(pfrom->cs_filter);
-            pfrom->fRelayTxes = fRelay; // set to true after we get the first filter* message
+            if (!vRecv.empty()) {
+                vRecv >> pfrom->fRelayTxes; // set to true after we get the first filter* message
+            } else {
+                pfrom->fRelayTxes = true;
+            }
         }
 
         // Change version
@@ -1729,12 +1733,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         CBloomFilter filter;
         vRecv >> filter;
 
+        LOCK(pfrom->cs_filter);
+
         if (!filter.IsWithinSizeConstraints()) {
             // There is no excuse for sending a too-large filter
             LOCK(cs_main);
             Misbehaving(pfrom->GetId(), 100);
         } else {
-            LOCK(pfrom->cs_filter);
             delete pfrom->pfilter;
             pfrom->pfilter = new CBloomFilter(filter);
             pfrom->pfilter->UpdateEmptyFull();
@@ -2095,6 +2100,12 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                 pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound);
             }
 
+            // Time to send but the peer has requested we not relay transactions.
+            if (fSendTrickle) {
+                LOCK(pto->cs_filter);
+                if (!pto->fRelayTxes) pto->setInventoryTxToSend.clear();
+            }
+
             // Respond to BIP35 mempool requests
             if (fSendTrickle && pto->fSendMempool) {
                 std::vector<uint256> vtxid;
@@ -2137,6 +2148,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                 // No reason to drain out at many times the network's capacity,
                 // especially since we have many peers and some will draw much shorter delays.
                 unsigned int nRelayedTransactions = 0;
+                LOCK(pto->cs_filter);
                 while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) {
                     // Fetch the top element from the heap
                     std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder);
@@ -2149,6 +2161,13 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                     if (pto->filterInventoryKnown.contains(hash)) {
                         continue;
                     }
+                    // Not in the mempool anymore? don't bother sending it.
+                    // todo: back port feerate filter.
+                    if (pto->pfilter) {
+                        CTransaction tx;
+                        if (!mempool.lookup(hash, tx)) continue;
+                        if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue;
+                    }
                     // Send
                     vInv.emplace_back(CInv(MSG_TX, hash));
                     nRelayedTransactions++;

From 35bc2a993e3fe78483ef555610a6593ba8edbef1 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 31 Dec 2020 13:34:53 -0300
Subject: [PATCH 16/32] Finished the switch CTransaction storage in mempool to
 CTransactionRef and introduced the timeLastMempoolReq coming from btc#8080.

---
 src/net.cpp            |  1 +
 src/net.h              |  3 ++
 src/net_processing.cpp | 29 +++++++--------
 src/txmempool.cpp      | 80 +++++++++++++++++++++++++++++++++++-------
 src/txmempool.h        | 20 +++++++++++
 5 files changed, 104 insertions(+), 29 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 1727074eb5e2..792ff0a05ed4 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2445,6 +2445,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     nNextInvSend = 0;
     fRelayTxes = false;
     pfilter = new CBloomFilter();
+    timeLastMempoolReq = 0;
     nPingNonceSent = 0;
     nPingUsecStart = 0;
     nPingUsecTime = 0;
diff --git a/src/net.h b/src/net.h
index 765fcf3a156f..365ff809886e 100644
--- a/src/net.h
+++ b/src/net.h
@@ -626,6 +626,9 @@ class CNode
     // Used for BIP35 mempool sending, also protected by cs_inventory
     bool fSendMempool;
 
+    // Last time a "MEMPOOL" request was serviced.
+    std::atomic<int64_t> timeLastMempoolReq{0};
+
     // Ping time measurement:
     // The pong reply we're expecting, or 0 if no pong expected.
     std::atomic<uint64_t> nPingNonceSent;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 48ccd01ec9eb..5f308867b767 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -864,11 +864,11 @@ void static ProcessGetData(CNode* pfrom, CConnman& connman, std::atomic<bool>& i
                 bool pushed = false;
 
                 if (inv.type == MSG_TX) {
-                    CTransaction tx;
-                    if (mempool.lookup(inv.hash, tx)) {
+                    auto txinfo = mempool.info(inv.hash);
+                    if (txinfo.tx) { // future: add timeLastMempoolReq check
                         CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
                         ss.reserve(1000);
-                        ss << tx;
+                        ss << *txinfo.tx;
                         connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, ss));
                         pushed = true;
                     }
@@ -1027,7 +1027,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
         std::string strSubVer;
         std::string cleanSubVer;
         int nStartingHeight = -1;
-        bool fRelay = true;
         vRecv >> nVersion >> nServiceInt >> nTime >> addrMe;
         nSendVersion = std::min(nVersion, PROTOCOL_VERSION);
         nServices = ServiceFlags(nServiceInt);
@@ -2108,21 +2107,18 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
 
             // Respond to BIP35 mempool requests
             if (fSendTrickle && pto->fSendMempool) {
-                std::vector<uint256> vtxid;
-                mempool.queryHashes(vtxid);
+                auto vtxinfo = mempool.infoAll();
                 pto->fSendMempool = false;
                 // future: back port fee filter rate
                 LOCK(pto->cs_filter);
 
-                for (const uint256& hash : vtxid) {
+                for (const auto& txinfo : vtxinfo) {
+                    const uint256& hash = txinfo.tx->GetHash();
                     CInv inv(MSG_TX, hash);
                     pto->setInventoryTxToSend.erase(hash);
                     // future: add fee filter check here..
                     if (pto->pfilter) {
-                        CTransaction tx;
-                        bool fInMemPool = mempool.lookup(hash, tx);
-                        if (!fInMemPool) continue; // another thread removed since queryHashes, maybe...
-                        if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue;
+                        if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
                     }
                     pto->filterInventoryKnown.insert(hash);
                     vInv.emplace_back(inv);
@@ -2131,6 +2127,7 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                         vInv.clear();
                     }
                 }
+                pto->timeLastMempoolReq = GetTime();
             }
 
             // Determine transactions to relay
@@ -2162,12 +2159,12 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
                         continue;
                     }
                     // Not in the mempool anymore? don't bother sending it.
-                    // todo: back port feerate filter.
-                    if (pto->pfilter) {
-                        CTransaction tx;
-                        if (!mempool.lookup(hash, tx)) continue;
-                        if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue;
+                    auto txinfo = mempool.info(hash);
+                    if (!txinfo.tx) {
+                        continue;
                     }
+                    // todo: back port feerate filter.
+                    if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
                     // Send
                     vInv.emplace_back(CInv(MSG_TX, hash));
                     nRelayedTransactions++;
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index cfda1f746c5a..277fa80522a9 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -829,25 +829,60 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb
 }
 
 namespace {
-class DepthAndScoreComparator
+    class DepthAndScoreComparator
+    {
+    public:
+        bool operator()(const CTxMemPool::indexed_transaction_set::const_iterator& a, const CTxMemPool::indexed_transaction_set::const_iterator& b) const
+        {
+            uint64_t counta = a->GetCountWithAncestors();
+            uint64_t countb = b->GetCountWithAncestors();
+            if (counta == countb) {
+                return CompareTxMemPoolEntryByScore()(*a, *b);
+            }
+            return counta < countb;
+        }
+    };
+}
+
+std::vector<CTxMemPool::indexed_transaction_set::const_iterator> CTxMemPool::GetSortedDepthAndScore() const
 {
-    CTxMemPool *mp;
-public:
-    DepthAndScoreComparator(CTxMemPool *mempool) : mp(mempool) {}
-    bool operator()(const uint256& a, const uint256& b) { return mp->CompareDepthAndScore(a, b); }
-};
+    std::vector<indexed_transaction_set::const_iterator> iters;
+    AssertLockHeld(cs);
+
+    iters.reserve(mapTx.size());
+
+    for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) {
+        iters.push_back(mi);
+    }
+    std::sort(iters.begin(), iters.end(), DepthAndScoreComparator());
+    return iters;
 }
 
 void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
 {
+    LOCK(cs);
+    auto iters = GetSortedDepthAndScore();
+
     vtxid.clear();
+    vtxid.reserve(mapTx.size());
 
+    for (auto it : iters) {
+        vtxid.emplace_back(it->GetTx().GetHash());
+    }
+}
+
+std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
+{
     LOCK(cs);
-    vtxid.reserve(mapTx.size());
-    for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi)
-        vtxid.push_back(mi->GetTx().GetHash());
+    auto iters = GetSortedDepthAndScore();
+
+    std::vector<TxMempoolInfo> ret;
+    ret.reserve(mapTx.size());
+    for (auto it : iters) {
+        ret.emplace_back(TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize())});
+    }
 
-    std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this));
+    return ret;
 }
 
 void CTxMemPool::getTransactions(std::set<uint256>& setTxid)
@@ -859,13 +894,32 @@ void CTxMemPool::getTransactions(std::set<uint256>& setTxid)
         setTxid.insert(mi->GetTx().GetHash());
 }
 
+std::shared_ptr<const CTransaction> CTxMemPool::get(const uint256& hash) const
+{
+    LOCK(cs);
+    indexed_transaction_set::const_iterator i = mapTx.find(hash);
+    if (i == mapTx.end())
+        return nullptr;
+    return i->GetSharedTx();
+}
+
 bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
+{
+    auto tx = get(hash);
+    if (tx) {
+        result = *tx;
+        return true;
+    }
+    return false;
+}
+
+TxMempoolInfo CTxMemPool::info(const uint256& hash) const
 {
     LOCK(cs);
     indexed_transaction_set::const_iterator i = mapTx.find(hash);
-    if (i == mapTx.end()) return false;
-    result = i->GetTx();
-    return true;
+    if (i == mapTx.end())
+        return TxMempoolInfo();
+    return TxMempoolInfo{i->GetSharedTx(), i->GetTime(), CFeeRate(i->GetFee(), i->GetTxSize())};
 }
 
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
diff --git a/src/txmempool.h b/src/txmempool.h
index 715720eac7f7..50bbc91aff22 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -286,6 +286,21 @@ struct ancestor_score {};
 
 class CBlockPolicyEstimator;
 
+/**
+ * Information about a mempool transaction.
+ */
+struct TxMempoolInfo
+{
+    /** The transaction itself */
+    std::shared_ptr<const CTransaction> tx;
+
+    /** Time the transaction entered the mempool. */
+    int64_t nTime;
+
+    /** Feerate of the transaction. */
+    CFeeRate feeRate;
+};
+
 /**
  * CTxMemPool stores valid-according-to-the-current-best-chain
  * transactions that may be included in the next block.
@@ -476,6 +491,8 @@ class CTxMemPool
     void UpdateParent(txiter entry, txiter parent, bool add);
     void UpdateChild(txiter entry, txiter child, bool add);
 
+    std::vector<indexed_transaction_set::const_iterator> GetSortedDepthAndScore() const;
+
 public:
     indirectmap<COutPoint, CTransactionRef> mapNextTx;
     std::map<uint256, std::pair<double, CAmount> > mapDeltas;
@@ -603,6 +620,9 @@ class CTxMemPool
     }
 
     bool lookup(uint256 hash, CTransaction& result) const;
+    CTransactionRef get(const uint256& hash) const;
+    TxMempoolInfo info(const uint256& hash) const;
+    std::vector<TxMempoolInfo> infoAll() const;
 
     /** Estimate fee rate needed to get into the next nBlocks
      *  If no answer can be given at nBlocks, return an estimate

From 54cf7c0107fe5b467f2e3fca6b69d9be7d0ce686 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 31 Dec 2020 13:54:09 -0300
Subject: [PATCH 17/32] Get rid of CTxMempool::lookup() entirely

Coming from btc@288d85ddf2e0a0c9d25a23db56052883170466d0
---
 src/test/policyestimator_tests.cpp | 18 +++++++++---------
 src/txmempool.cpp                  | 18 ++++--------------
 src/txmempool.h                    |  1 -
 src/validation.cpp                 |  5 ++++-
 4 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index 7c1bbea41635..4484f478b60d 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -67,9 +67,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
             // 9/10 blocks add 2nd highest and so on until ...
             // 1/10 blocks add lowest fee transactions
             while (txHashes[9-h].size()) {
-                CTransaction btx;
-                if (mpool.lookup(txHashes[9-h].back(), btx))
-                    block.push_back(std::make_shared<const CTransaction>(btx));
+                CTransactionRef ptx = mpool.get(txHashes[9-h].back());
+                if (ptx)
+                    block.emplace_back(ptx);
                 txHashes[9-h].pop_back();
             }
         }
@@ -144,9 +144,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     // Estimates should still not be below original
     for (int j = 0; j < 10; j++) {
         while(txHashes[j].size()) {
-            CTransaction btx;
-            if (mpool.lookup(txHashes[j].back(), btx))
-                block.push_back(std::make_shared<const CTransaction>(btx));
+            CTransactionRef ptx = mpool.get(txHashes[j].back());
+            if (ptx)
+                block.emplace_back(ptx);
             txHashes[j].pop_back();
         }
     }
@@ -164,9 +164,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                 tx.vin[0].prevout.n = 10000*blocknum+100*j+k;
                 uint256 hash = tx.GetHash();
                 mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool));
-                CTransaction btx;
-                if (mpool.lookup(hash, btx))
-                    block.push_back(std::make_shared<const CTransaction>(btx));
+                CTransactionRef ptx = mpool.get(hash);
+                if (ptx)
+                    block.emplace_back(ptx);
             }
         }
         mpool.removeForBlock(block, ++blocknum, dummyConflicted);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 277fa80522a9..66e7e5389e70 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -903,16 +903,6 @@ std::shared_ptr<const CTransaction> CTxMemPool::get(const uint256& hash) const
     return i->GetSharedTx();
 }
 
-bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
-{
-    auto tx = get(hash);
-    if (tx) {
-        result = *tx;
-        return true;
-    }
-    return false;
-}
-
 TxMempoolInfo CTxMemPool::info(const uint256& hash) const
 {
     LOCK(cs);
@@ -1038,10 +1028,10 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint& outpoint, Coin& coin) const
     // If an entry in the mempool exists, always return that one, as it's guaranteed to never
     // conflict with the underlying cache, and it cannot have pruned entries (as it contains full)
     // transactions. First checking the underlying cache risks returning a pruned entry instead.
-    CTransaction tx;
-    if (mempool.lookup(outpoint.hash, tx)) {
-        if (outpoint.n < tx.vout.size()) {
-            coin = Coin(tx.vout[outpoint.n], MEMPOOL_HEIGHT, false, false);
+    CTransactionRef ptx = mempool.get(outpoint.hash);
+    if (ptx) {
+        if (outpoint.n < ptx->vout.size()) {
+            coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false, false);
             return true;
         } else {
             return false;
diff --git a/src/txmempool.h b/src/txmempool.h
index 50bbc91aff22..0f377b0c5c60 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -619,7 +619,6 @@ class CTxMemPool
         return (it != mapTx.end() && outpoint.n < it->GetTx().vout.size());
     }
 
-    bool lookup(uint256 hash, CTransaction& result) const;
     CTransactionRef get(const uint256& hash) const;
     TxMempoolInfo info(const uint256& hash) const;
     std::vector<TxMempoolInfo> infoAll() const;
diff --git a/src/validation.cpp b/src/validation.cpp
index 4da8cb15c5f4..29c981ac0f66 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -648,7 +648,10 @@ bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock
     LOCK(cs_main);
 
     if (!blockIndex) {
-        if (mempool.lookup(hash, txOut)) {
+
+        CTransactionRef ptx = mempool.get(hash);
+        if (ptx) {
+            txOut = *ptx;
             return true;
         }
 

From e51c4b82f0748c9f826f72c03c3d0868bf7aebf1 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <sipa@ulyssis.org>
Date: Wed, 13 Jan 2021 12:52:14 -0300
Subject: [PATCH 18/32] Bypass removeRecursive in removeForReorg

---
 src/txmempool.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 66e7e5389e70..5b3f39723532 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -528,11 +528,11 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
     AssertLockHeld(cs_main);
     // Remove transactions spending a coinbase which are now immature and no-longer-final transactions
     LOCK(cs);
-    std::list<CTransaction> transactionsToRemove;
+    setEntries txToRemove;
     for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
         const CTransaction& tx = it->GetTx();
         if (!CheckFinalTx(tx, flags)) {
-            transactionsToRemove.push_back(tx);
+            txToRemove.insert(it);
         } else if (it->GetSpendsCoinbaseOrCoinstake()) {
             for (const CTxIn& txin : tx.vin) {
                 indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
@@ -541,16 +541,17 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
                 const Coin &coin = pcoins->AccessCoin(txin.prevout);
                 if (nCheckFrequency != 0) assert(!coin.IsSpent());
                 if (coin.IsSpent() || ((coin.IsCoinBase() || coin.IsCoinStake()) && ((signed long)nMemPoolHeight) - coin.nHeight < Params().GetConsensus().nCoinbaseMaturity)) {
-                    transactionsToRemove.push_back(tx);
+                    txToRemove.insert(it);
                     break;
                 }
             }
         }
     }
-    for (const CTransaction& tx : transactionsToRemove) {
-        std::list<CTransactionRef> removed;
-        removeRecursive(tx, removed);
+    setEntries setAllRemoves;
+    for (txiter it : txToRemove) {
+        CalculateDescendants(it, setAllRemoves);
     }
+    RemoveStaged(setAllRemoves, false);
 }
 
 void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)

From 4f672c273f67b5b148bb78552b5c565ef47084e2 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <sipa@ulyssis.org>
Date: Wed, 13 Jan 2021 13:16:27 -0300
Subject: [PATCH 19/32] Make removed and conflicted arguments optional to
 remove

---
 src/test/mempool_tests.cpp         | 27 ++++++++++++---------------
 src/test/policyestimator_tests.cpp | 11 +++++------
 src/txmempool.cpp                  | 16 +++++++++-------
 src/txmempool.h                    |  7 ++++---
 src/validation.cpp                 |  5 ++---
 5 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index b4a545a182b9..bdf967d76511 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -56,12 +56,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     std::list<CTransactionRef> removed;
 
     // Nothing in pool, remove should do nothing:
-    testPool.removeRecursive(txParent, removed);
+    testPool.removeRecursive(txParent, &removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
 
     // Just the parent:
     testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
-    testPool.removeRecursive(txParent, removed);
+    testPool.removeRecursive(txParent, &removed);
     BOOST_CHECK_EQUAL(removed.size(), 1);
     removed.clear();
     
@@ -73,16 +73,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
         testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
     }
     // Remove Child[0], GrandChild[0] should be removed:
-    testPool.removeRecursive(txChild[0], removed);
+    testPool.removeRecursive(txChild[0], &removed);
     BOOST_CHECK_EQUAL(removed.size(), 2);
     removed.clear();
     // ... make sure grandchild and child are gone:
-    testPool.removeRecursive(txGrandChild[0], removed);
+    testPool.removeRecursive(txGrandChild[0], &removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
-    testPool.removeRecursive(txChild[0], removed);
+    testPool.removeRecursive(txChild[0], &removed);
     BOOST_CHECK_EQUAL(removed.size(), 0);
     // Remove parent, all children/grandchildren should go:
-    testPool.removeRecursive(txParent, removed);
+    testPool.removeRecursive(txParent, &removed);
     BOOST_CHECK_EQUAL(removed.size(), 5);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
     removed.clear();
@@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
     }
     // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be
     // put into the mempool (maybe because it is non-standard):
-    testPool.removeRecursive(txParent, removed);
+    testPool.removeRecursive(txParent, &removed);
     BOOST_CHECK_EQUAL(removed.size(), 6);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
     removed.clear();
@@ -279,12 +279,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     BOOST_CHECK_EQUAL(pool.size(), 10);
 
     // Now try removing tx10 and verify the sort order returns to normal
-    std::list<CTransactionRef> removed;
-    pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed);
+    pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx());
     CheckSort<descendant_score>(pool, snapshotOrder);
 
-    pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed);
-    pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed);
+    pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx());
+    pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx());
     /* Now check the sort on the mining score index.
      * Final order should be:
      *
@@ -416,8 +415,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
     /* after tx6 is mined, tx7 should move up in the sort */
     std::vector<CTransactionRef> vtx;
     vtx.emplace_back(MakeTransactionRef(tx6));
-    std::list<CTransactionRef> dummy;
-    pool.removeForBlock(vtx, 1, dummy, false);
+    pool.removeForBlock(vtx, 1, nullptr, false);
 
     sortedOrder.erase(sortedOrder.begin()+1);
     // Ties are broken by hash
@@ -556,12 +554,11 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool));
 
     std::vector<CTransactionRef> vtx;
-    std::list<CTransactionRef> conflicts;
     SetMockTime(42);
     SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
     // ... we should keep the same min fee until we get a block
-    pool.removeForBlock(vtx, 1, conflicts);
+    pool.removeForBlock(vtx, 1);
     SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2);
     // ... then feerate should drop 1/2 each halflife
diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index 4484f478b60d..16e7bb27b7be 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -38,7 +38,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     for (unsigned int i = 0; i < 128; i++)
         garbage.push_back('X');
     CMutableTransaction tx;
-    std::list<CTransactionRef> dummyConflicted;
     tx.vin.resize(1);
     tx.vin[0].scriptSig = garbage;
     tx.vout.resize(1);
@@ -73,7 +72,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                 txHashes[9-h].pop_back();
             }
         }
-        mpool.removeForBlock(block, ++blocknum, dummyConflicted);
+        mpool.removeForBlock(block, ++blocknum);
         block.clear();
         if (blocknum == 30) {
             // At this point we should need to combine 5 buckets to get enough data points
@@ -112,7 +111,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     // Mine 50 more blocks with no transactions happening, estimates shouldn't change
     // We haven't decayed the moving average enough so we still have enough data points in every bucket
     while (blocknum < 250)
-        mpool.removeForBlock(block, ++blocknum, dummyConflicted);
+        mpool.removeForBlock(block, ++blocknum);
 
     for (int i = 1; i < 10;i++) {
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee);
@@ -131,7 +130,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                 txHashes[j].push_back(hash);
             }
         }
-        mpool.removeForBlock(block, ++blocknum, dummyConflicted);
+        mpool.removeForBlock(block, ++blocknum);
     }
 
     int answerFound;
@@ -150,7 +149,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
             txHashes[j].pop_back();
         }
     }
-    mpool.removeForBlock(block, 265, dummyConflicted);
+    mpool.removeForBlock(block, 265);
     block.clear();
     for (int i = 1; i < 10;i++) {
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
@@ -169,7 +168,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
                     block.emplace_back(ptx);
             }
         }
-        mpool.removeForBlock(block, ++blocknum, dummyConflicted);
+        mpool.removeForBlock(block, ++blocknum);
         block.clear();
     }
     for (int i = 1; i < 10; i++) {
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 5b3f39723532..4f3197364cb2 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -489,7 +489,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
     }
 }
 
-void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransactionRef>& removed)
+void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransactionRef>* removed)
 {
     // Remove transaction from memory pool
     {
@@ -516,8 +516,10 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransact
         for (const txiter& it : txToRemove) {
             CalculateDescendants(it, setAllRemoves);
         }
-        for (const txiter& it : setAllRemoves) {
-            removed.emplace_back(it->GetSharedTx());
+        if (removed) {
+            for (const txiter& it : setAllRemoves) {
+                removed->emplace_back(it->GetSharedTx());
+            }
         }
         RemoveStaged(setAllRemoves, false);
     }
@@ -574,12 +576,11 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
         }
     }
     for (const CTransaction& tx : transactionsToRemove) {
-        std::list<CTransactionRef> removed;
-        removeRecursive(tx, removed);
+        removeRecursive(tx);
     }
 }
 
-void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionRef>& removed)
+void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionRef>* removed)
 {
     // Remove transactions which depend on inputs of tx, recursively
     std::list<CTransaction> result;
@@ -610,7 +611,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
 /**
  * Called when a block is connected. Removes from mempool and updates the miner fee estimator.
  */
-void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight, std::list<CTransactionRef>& conflicts, bool fCurrentEstimate)
+void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
+                                std::list<CTransactionRef>* conflicts, bool fCurrentEstimate)
 {
     LOCK(cs);
     std::vector<CTxMemPoolEntry> entries;
diff --git a/src/txmempool.h b/src/txmempool.h
index 0f377b0c5c60..3129f355edf8 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -520,11 +520,12 @@ class CTxMemPool
     // then invoke the second version.
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true);
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
-    void removeRecursive(const CTransaction& tx, std::list<CTransactionRef>& removed);
+    void removeRecursive(const CTransaction& tx, std::list<CTransactionRef>* removed = nullptr);
     void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags);
     void removeWithAnchor(const uint256& invalidRoot);
-    void removeConflicts(const CTransaction& tx, std::list<CTransactionRef>& removed);
-    void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight, std::list<CTransactionRef>& conflicts, bool fCurrentEstimate = true);
+    void removeConflicts(const CTransaction& tx, std::list<CTransactionRef>* removed = nullptr);
+    void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
+                        std::list<CTransactionRef>* conflicts = nullptr, bool fCurrentEstimate = true);
     void clear();
     void _clear();  // lock-free
     bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);
diff --git a/src/validation.cpp b/src/validation.cpp
index 29c981ac0f66..17a19e0dc6b3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1966,10 +1966,9 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
     std::vector<uint256> vHashUpdate;
     for (const auto& tx : block.vtx) {
         // ignore validation errors in resurrected transactions
-        std::list<CTransactionRef> removed;
         CValidationState stateDummy;
         if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) {
-            mempool.removeRecursive(*tx, removed);
+            mempool.removeRecursive(*tx);
         } else if (mempool.exists(tx->GetHash())) {
             vHashUpdate.push_back(tx->GetHash());
         }
@@ -2086,7 +2085,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st
     LogPrint(BCLog::BENCH, "  - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001);
 
     // Remove conflicting transactions from the mempool.
-    mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, connectTrace.txConflicted, !IsInitialBlockDownload());
+    mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload());
     // Update chainActive & related variables.
     UpdateTip(pindexNew);
     // Update MN manager cache

From 9979f3d4deb7534b92908ae2c49c3e28c7a46baf Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Wed, 13 Jan 2021 13:22:27 -0300
Subject: [PATCH 20/32] [mempool] move removed and conflicts transaction ref
 list to vector.

---
 src/test/mempool_tests.cpp | 2 +-
 src/txmempool.cpp          | 6 +++---
 src/txmempool.h            | 6 +++---
 src/validation.cpp         | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index bdf967d76511..9ed326af740a 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
 
 
     CTxMemPool testPool(CFeeRate(0));
-    std::list<CTransactionRef> removed;
+    std::vector<CTransactionRef> removed;
 
     // Nothing in pool, remove should do nothing:
     testPool.removeRecursive(txParent, &removed);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 4f3197364cb2..2e3c4a1e5349 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -489,7 +489,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
     }
 }
 
-void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransactionRef>* removed)
+void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vector<CTransactionRef>* removed)
 {
     // Remove transaction from memory pool
     {
@@ -580,7 +580,7 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
     }
 }
 
-void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionRef>* removed)
+void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector<CTransactionRef>* removed)
 {
     // Remove transactions which depend on inputs of tx, recursively
     std::list<CTransaction> result;
@@ -612,7 +612,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
  * Called when a block is connected. Removes from mempool and updates the miner fee estimator.
  */
 void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
-                                std::list<CTransactionRef>* conflicts, bool fCurrentEstimate)
+                                std::vector<CTransactionRef>* conflicts, bool fCurrentEstimate)
 {
     LOCK(cs);
     std::vector<CTxMemPoolEntry> entries;
diff --git a/src/txmempool.h b/src/txmempool.h
index 3129f355edf8..b108c04a7677 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -520,12 +520,12 @@ class CTxMemPool
     // then invoke the second version.
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true);
     bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
-    void removeRecursive(const CTransaction& tx, std::list<CTransactionRef>* removed = nullptr);
+    void removeRecursive(const CTransaction& tx, std::vector<CTransactionRef>* removed = nullptr);
     void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags);
     void removeWithAnchor(const uint256& invalidRoot);
-    void removeConflicts(const CTransaction& tx, std::list<CTransactionRef>* removed = nullptr);
+    void removeConflicts(const CTransaction& tx, std::vector<CTransactionRef>* removed = nullptr);
     void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
-                        std::list<CTransactionRef>* conflicts = nullptr, bool fCurrentEstimate = true);
+                        std::vector<CTransactionRef>* conflicts = nullptr, bool fCurrentEstimate = true);
     void clear();
     void _clear();  // lock-free
     bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);
diff --git a/src/validation.cpp b/src/validation.cpp
index 17a19e0dc6b3..36c208a6f92e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2020,7 +2020,7 @@ static int64_t nTimePostConnect = 0;
  * applied to the UTXO state as a part of a single ActivateBestChainStep call.
  */
 struct ConnectTrace {
-    std::list<CTransactionRef> txConflicted;
+    std::vector<CTransactionRef> txConflicted;
     std::vector<std::pair<CBlockIndex*, std::shared_ptr<const CBlock> > > blocksConnected;
 };
 

From 6bbc6a90424bac4c865423f96d56cbd33dd4987c Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter.wuille@gmail.com>
Date: Sun, 31 Jul 2016 20:53:17 +0200
Subject: [PATCH 21/32] Add feedelta to TxMempoolInfo

---
 src/txmempool.cpp | 8 ++++++--
 src/txmempool.h   | 3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 2e3c4a1e5349..0ee9e728a59f 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -874,6 +874,10 @@ void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
     }
 }
 
+static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) {
+    return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize()), it->GetModifiedFee() - it->GetFee()};
+}
+
 std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
 {
     LOCK(cs);
@@ -882,7 +886,7 @@ std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
     std::vector<TxMempoolInfo> ret;
     ret.reserve(mapTx.size());
     for (auto it : iters) {
-        ret.emplace_back(TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize())});
+        ret.emplace_back(GetInfo(it));
     }
 
     return ret;
@@ -912,7 +916,7 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
     indexed_transaction_set::const_iterator i = mapTx.find(hash);
     if (i == mapTx.end())
         return TxMempoolInfo();
-    return TxMempoolInfo{i->GetSharedTx(), i->GetTime(), CFeeRate(i->GetFee(), i->GetTxSize())};
+    return GetInfo(i);
 }
 
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
diff --git a/src/txmempool.h b/src/txmempool.h
index b108c04a7677..d235013d2cd8 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -299,6 +299,9 @@ struct TxMempoolInfo
 
     /** Feerate of the transaction. */
     CFeeRate feeRate;
+
+    /** The fee delta. */
+    int64_t nFeeDelta;
 };
 
 /**

From 44c635d7adec03cb51f951dccbb087eef3a805ac Mon Sep 17 00:00:00 2001
From: Pieter Wuille <sipa@ulyssis.org>
Date: Wed, 13 Jan 2021 18:46:00 -0300
Subject: [PATCH 22/32] Add AcceptToMemoryPoolWithTime function

---
 src/validation.cpp | 17 ++++++++++++-----
 src/validation.h   |  5 +++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 36c208a6f92e..9006f5ab338d 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -309,7 +309,7 @@ std::string FormatStateMessage(const CValidationState &state)
 }
 
 bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransactionRef& _tx, bool fLimitFree,
-                              bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees,
+                              bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees,
                               std::vector<COutPoint>& coins_to_uncache)
 {
     AssertLockHeld(cs_main);
@@ -495,7 +495,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
             }
         }
 
-        CTxMemPoolEntry entry(_tx, nFees, GetTime(), dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps);
+        CTxMemPoolEntry entry(_tx, nFees, nAcceptTime, dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps);
         unsigned int nSize = entry.GetTxSize();
 
         // Don't accept it if it can't get into a block
@@ -611,11 +611,11 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
     return true;
 }
 
-bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef& tx, bool fLimitFree,
-                        bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool fIgnoreFees)
+bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef& tx, bool fLimitFree,
+                        bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool fIgnoreFees)
 {
     std::vector<COutPoint> coins_to_uncache;
-    bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, fOverrideMempoolLimit, fRejectAbsurdFee, fIgnoreFees, coins_to_uncache);
+    bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, fOverrideMempoolLimit, fRejectAbsurdFee, fIgnoreFees, coins_to_uncache);
     if (!res) {
         for (const COutPoint& outpoint: coins_to_uncache)
             pcoinsTip->Uncache(outpoint);
@@ -626,6 +626,13 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
     return res;
 }
 
+bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransactionRef& tx,
+                        bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit,
+                        bool fRejectInsaneFee, bool ignoreFees)
+{
+    return AcceptToMemoryPoolWithTime(pool, state, tx, fLimitFree, pfMissingInputs, GetTime(), fOverrideMempoolLimit, fRejectInsaneFee, ignoreFees);
+}
+
 bool GetOutput(const uint256& hash, unsigned int index, CValidationState& state, CTxOut& out)
 {
     CTransaction txPrev;
diff --git a/src/validation.h b/src/validation.h
index fa4c987ce318..9e7ee1e7082e 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -216,6 +216,11 @@ void FlushStateToDisk();
 /** (try to) add transaction to memory pool **/
 bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransactionRef& tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit = false, bool fRejectInsaneFee = false, bool ignoreFees = false);
 
+/** (try to) add transaction to memory pool with a specified acceptance time **/
+bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree,
+                                bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit = false,
+                                bool fRejectInsaneFee = false, bool ignoreFees = false);
+
 /** Convert CValidationState to a human-readable message for logging */
 std::string FormatStateMessage(const CValidationState &state);
 

From 8e522264c14532c6699b08bc89310b6afcddceef Mon Sep 17 00:00:00 2001
From: Pieter Wuille <sipa@ulyssis.org>
Date: Thu, 14 Jan 2021 00:48:56 -0300
Subject: [PATCH 23/32] Add DumpMempool and LoadMempool

---
 src/init.cpp       |   3 ++
 src/validation.cpp | 112 +++++++++++++++++++++++++++++++++++++++++++++
 src/validation.h   |   6 +++
 3 files changed, 121 insertions(+)

diff --git a/src/init.cpp b/src/init.cpp
index a0f200660cde..7e843d3414af 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -245,6 +245,7 @@ void PrepareShutdown()
     DumpBudgets(g_budgetman);
     DumpMasternodePayments();
     UnregisterNodeSignals(GetNodeSignals());
+    DumpMempool();
 
     // After everything has been shut down, but before things get flushed, stop the
     // CScheduler/checkqueue threadGroup
@@ -700,6 +701,8 @@ void ThreadImport(std::vector<fs::path> vImportFiles)
         LogPrintf("Stopping after block import\n");
         StartShutdown();
     }
+
+    LoadMempool();
 }
 
 /** Sanity checks
diff --git a/src/validation.cpp b/src/validation.cpp
index 9006f5ab338d..10abe2c229a7 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4096,6 +4096,118 @@ std::string CBlockFileInfo::ToString() const
     return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast));
 }
 
+static const uint64_t MEMPOOL_DUMP_VERSION = 1;
+
+bool LoadMempool(void)
+{
+    int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60;
+    FILE* filestr = fopen((GetDataDir() / "mempool.dat").string().c_str(), "r");
+    CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
+    if (file.IsNull()) {
+        LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n");
+        return false;
+    }
+
+    int64_t count = 0;
+    int64_t skipped = 0;
+    int64_t failed = 0;
+    int64_t nNow = GetTime();
+
+    try {
+        uint64_t version;
+        file >> version;
+        if (version != MEMPOOL_DUMP_VERSION) {
+            return false;
+        }
+        uint64_t num;
+        file >> num;
+        double prioritydummy = 0;
+        while (num--) {
+            CTransactionRef tx;
+            int64_t nTime;
+            int64_t nFeeDelta;
+            file >> tx;
+            file >> nTime;
+            file >> nFeeDelta;
+
+            CAmount amountdelta = nFeeDelta;
+            if (amountdelta) {
+                mempool.PrioritiseTransaction(tx->GetHash(), tx->GetHash().ToString(), prioritydummy, amountdelta);
+            }
+            CValidationState state;
+            if (nTime + nExpiryTimeout > nNow) {
+                LOCK(cs_main);
+                AcceptToMemoryPoolWithTime(mempool, state, tx, true, NULL, nTime);
+                if (state.IsValid()) {
+                    ++count;
+                } else {
+                    ++failed;
+                }
+            } else {
+                ++skipped;
+            }
+        }
+        std::map<uint256, CAmount> mapDeltas;
+        file >> mapDeltas;
+
+        for (const auto& i : mapDeltas) {
+            mempool.PrioritiseTransaction(i.first, i.first.ToString(), prioritydummy, i.second);
+        }
+    } catch (const std::exception& e) {
+        LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what());
+        return false;
+    }
+
+    LogPrintf("Imported mempool transactions from disk: %i successes, %i failed, %i expired\n", count, failed, skipped);
+    return true;
+}
+
+void DumpMempool(void)
+{
+    int64_t start = GetTimeMicros();
+
+    std::map<uint256, CAmount> mapDeltas;
+    std::vector<TxMempoolInfo> vinfo;
+
+    {
+        LOCK(mempool.cs);
+        for (const auto &i : mempool.mapDeltas) {
+            mapDeltas[i.first] = i.second.first;
+        }
+        vinfo = mempool.infoAll();
+    }
+
+    int64_t mid = GetTimeMicros();
+
+    try {
+        FILE* filestr = fopen((GetDataDir() / "mempool.dat.new").string().c_str(), "w");
+        if (!filestr) {
+            return;
+        }
+
+        CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
+
+        uint64_t version = MEMPOOL_DUMP_VERSION;
+        file << version;
+
+        file << (uint64_t)vinfo.size();
+        for (const auto& i : vinfo) {
+            file << i.tx;
+            file << (int64_t)i.nTime;
+            file << (int64_t)i.nFeeDelta;
+            mapDeltas.erase(i.tx->GetHash());
+        }
+
+        file << mapDeltas;
+        FileCommit(file.Get());
+        file.fclose();
+        RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat");
+        int64_t last = GetTimeMicros();
+        LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*0.000001, (last-mid)*0.000001);
+    } catch (const std::exception& e) {
+        LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what());
+    }
+}
 
 class CMainCleanup
 {
diff --git a/src/validation.h b/src/validation.h
index 9e7ee1e7082e..119b1de22c2b 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -397,4 +397,10 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101;
 /** Transaction conflicts with a transaction already known */
 static const unsigned int REJECT_CONFLICT = 0x102;
 
+/** Dump the mempool to disk. */
+void DumpMempool();
+
+/** Load the mempool from disk. */
+bool LoadMempool();
+
 #endif // BITCOIN_MAIN_H

From f0c22558f4d76357ce5658cf31dbd47b57d65a90 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 14 Jan 2021 00:50:29 -0300
Subject: [PATCH 24/32] Add mempool.dat to doc/files.md

---
 doc/files.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/files.md b/doc/files.md
index 18b672c63a24..7815643abb0b 100644
--- a/doc/files.md
+++ b/doc/files.md
@@ -11,6 +11,7 @@ database/*          | BDB database environment; only used for wallet since 0.8.0
 db.log              | wallet database log file; moved to wallets/ directory on new installs since 0.16.0
 debug.log           | contains debug information and general logging generated by pivxd or pivx-qt
 fee_estimates.dat   | stores statistics used to estimate minimum transaction fees and priorities required for confirmation; since 0.10.0
+mempool.dat         | dump of the mempool's transactions; since 5.0.2
 budget.dat          | stores data for budget objects
 masternode.conf     | contains configuration settings for remote masternodes
 mncache.dat         | stores data for masternode list

From c0a0e81d898adaaf7e16e98b912fb04be04a11c9 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 14 Jan 2021 11:03:08 -0300
Subject: [PATCH 25/32] Moving TxMempoolInfo tx member to CTransactionRef

---
 src/txmempool.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/txmempool.h b/src/txmempool.h
index d235013d2cd8..dca6a3939405 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -292,7 +292,7 @@ class CBlockPolicyEstimator;
 struct TxMempoolInfo
 {
     /** The transaction itself */
-    std::shared_ptr<const CTransaction> tx;
+    CTransactionRef tx;
 
     /** Time the transaction entered the mempool. */
     int64_t nTime;

From e60da988df037c8542ff6bdc5192dc494b3ac846 Mon Sep 17 00:00:00 2001
From: Jonas Schnelli <dev@jonasschnelli.ch>
Date: Thu, 22 Dec 2016 20:11:26 +0100
Subject: [PATCH 26/32] Allow shutdown during LoadMempool, dump only when
 necessary

---
 src/init.cpp       | 5 ++++-
 src/validation.cpp | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/init.cpp b/src/init.cpp
index 7e843d3414af..309538fe6d20 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -155,6 +155,7 @@ CClientUIInterface uiInterface;  // Declared but not defined in guiinterface.h
 //
 
 volatile bool fRequestShutdown = false;
+std::atomic<bool> fDumpMempoolLater(false);
 
 void StartShutdown()
 {
@@ -245,7 +246,8 @@ void PrepareShutdown()
     DumpBudgets(g_budgetman);
     DumpMasternodePayments();
     UnregisterNodeSignals(GetNodeSignals());
-    DumpMempool();
+    if (fDumpMempoolLater)
+        DumpMempool();
 
     // After everything has been shut down, but before things get flushed, stop the
     // CScheduler/checkqueue threadGroup
@@ -703,6 +705,7 @@ void ThreadImport(std::vector<fs::path> vImportFiles)
     }
 
     LoadMempool();
+    fDumpMempoolLater = !fRequestShutdown;
 }
 
 /** Sanity checks
diff --git a/src/validation.cpp b/src/validation.cpp
index 10abe2c229a7..b2cb3df078ec 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4146,6 +4146,8 @@ bool LoadMempool(void)
             } else {
                 ++skipped;
             }
+            if (ShutdownRequested())
+                return false;
         }
         std::map<uint256, CAmount> mapDeltas;
         file >> mapDeltas;

From 5d949deee2cb64812fdea85cb24a6bb595a8a811 Mon Sep 17 00:00:00 2001
From: Jonas Schnelli <dev@jonasschnelli.ch>
Date: Thu, 14 Jan 2021 11:15:10 -0300
Subject: [PATCH 27/32] [Qt] Do proper shutdown

---
 src/qt/pivx.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp
index ef99ca8bcc8f..b80003d535e6 100644
--- a/src/qt/pivx.cpp
+++ b/src/qt/pivx.cpp
@@ -464,6 +464,8 @@ void BitcoinApplication::requestShutdown()
     // Show a simple window indicating shutdown status
     ShutdownWindow::showShutdownWindow(window);
 
+    StartShutdown();
+
     // Request shutdown from core thread
     Q_EMIT requestedShutdown();
 }

From 4f26a4e0f6048fa2fb45a6808558501353f5d838 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Thu, 14 Jan 2021 11:33:17 -0300
Subject: [PATCH 28/32] Control mempool persistence using a command line
 parameter.

---
 src/init.cpp     | 10 +++++++---
 src/validation.h |  2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index 309538fe6d20..1ad82f29385c 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -246,8 +246,9 @@ void PrepareShutdown()
     DumpBudgets(g_budgetman);
     DumpMasternodePayments();
     UnregisterNodeSignals(GetNodeSignals());
-    if (fDumpMempoolLater)
+    if (fDumpMempoolLater && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
         DumpMempool();
+    }
 
     // After everything has been shut down, but before things get flushed, stop the
     // CScheduler/checkqueue threadGroup
@@ -421,6 +422,7 @@ std::string HelpMessage(HelpMessageMode mode)
     strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
     strUsage += HelpMessageOpt("-maxmempool=<n>", strprintf(_("Keep the transaction memory pool below <n> megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE));
     strUsage += HelpMessageOpt("-mempoolexpiry=<n>", strprintf(_("Do not keep transactions in the mempool longer than <n> hours (default: %u)"), DEFAULT_MEMPOOL_EXPIRY));
+    strUsage += HelpMessageOpt("-persistmempool", strprintf(_("Whether to save the mempool on shutdown and load on restart (default: %u)"), DEFAULT_PERSIST_MEMPOOL));
     strUsage += HelpMessageOpt("-par=<n>", strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS));
 #ifndef WIN32
     strUsage += HelpMessageOpt("-pid=<file>", strprintf(_("Specify pid file (default: %s)"), PIVX_PID_FILENAME));
@@ -704,8 +706,10 @@ void ThreadImport(std::vector<fs::path> vImportFiles)
         StartShutdown();
     }
 
-    LoadMempool();
-    fDumpMempoolLater = !fRequestShutdown;
+    if (gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
+        LoadMempool();
+        fDumpMempoolLater = !fRequestShutdown;
+    }
 }
 
 /** Sanity checks
diff --git a/src/validation.h b/src/validation.h
index 119b1de22c2b..b33810d6714f 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -57,6 +57,8 @@ static const unsigned int DEFAULT_ANCESTOR_LIMIT = 25;
 static const unsigned int DEFAULT_ANCESTOR_SIZE_LIMIT = 101;
 /** Default for -banscore */
 static const int DEFAULT_BANSCORE_THRESHOLD = 100;
+/** Default for -persistmempool */
+static const bool DEFAULT_PERSIST_MEMPOOL = true;
 /** Default for -limitdescendantcount, max number of in-mempool descendants */
 static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25;
 /** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */

From c6d45c6f6be33e0940d71ad623a20f7f9b75845d Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Sun, 17 Jan 2021 18:26:47 -0300
Subject: [PATCH 29/32] Adapting and connecting mempool_persist.py functional
 test to the test runner.

---
 test/functional/mempool_persist.py | 75 ++++++++++++++----------------
 test/functional/test_runner.py     |  2 +-
 2 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py
index 83017a3f547a..52fa6d3e57d3 100755
--- a/test/functional/mempool_persist.py
+++ b/test/functional/mempool_persist.py
@@ -1,16 +1,16 @@
 #!/usr/bin/env python3
 # Copyright (c) 2014-2017 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+# file COPYING or https://www.opensource.org/licenses/mit-license.php.
 """Test mempool persistence.
 
 By default, bitcoind will dump mempool on shutdown and
 then reload it on startup. This can be overridden with
-the -persistmempool=0 command line option.
+the -persistmempool=false command line option.
 
 Test is as follows:
 
-  - start node0, node1 and node2. node1 has -persistmempool=0
+  - start node0, node1 and node2. node1 has -persistmempool=false
   - create 5 transactions on node2 to its own address. Note that these
     are not sent to node0 or node1 addresses because we don't want
     them to be saved in the wallet.
@@ -20,23 +20,16 @@
     in its mempool. Shutdown node0. This tests that by default the
     mempool is persistent.
   - startup node1. Verify that its mempool is empty. Shutdown node1.
-    This tests that with -persistmempool=0, the mempool is not
+    This tests that with -persistmempool=false, the mempool is not
     dumped to disk when the node is shut down.
-  - Restart node0 with -persistmempool=0. Verify that its mempool is
-    empty. Shutdown node0. This tests that with -persistmempool=0,
+  - Restart node0 with -persistmempool=false. Verify that its mempool is
+    empty. Shutdown node0. This tests that with -persistmempool=false,
     the mempool is not loaded from disk on start up.
-  - Restart node0 with -persistmempool. Verify that it has 5
-    transactions in its mempool. This tests that -persistmempool=0
+  - Restart node0 with -persistmempool=true. Verify that it has 5
+    transactions in its mempool. This tests that -persistmempool=false
     does not overwrite a previously valid mempool stored on disk.
-  - Remove node0 mempool.dat and verify savemempool RPC recreates it
-    and verify that node1 can load it and has 5 transaction in its
-    mempool.
-  - Verify that savemempool throws when the RPC is called if
-    node1 can't write to disk.
 
 """
-import os
-import time
 
 from test_framework.test_framework import PivxTestFramework
 from test_framework.util import *
@@ -64,19 +57,19 @@ def run_test(self):
         assert_equal(len(self.nodes[0].getrawmempool()), 5)
         assert_equal(len(self.nodes[1].getrawmempool()), 5)
 
-        self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.")
+        self.log.debug("Stop-start node0 and node1. Verify that node0 has the transactions in its mempool and node1 does not.")
         self.stop_nodes()
         self.start_node(1)  # Give this one a head-start, so we can be "extra-sure" that it didn't load anything later
         self.start_node(0)
         self.start_node(2)
-        # Give bitcoind a second to reload the mempool
+        # Give pivxd a second to reload the mempool
         wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5, timeout=1)
         wait_until(lambda: len(self.nodes[2].getrawmempool()) == 5, timeout=1)
-        # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now:
+        # The others loaded their mempool. If node_1 loaded anything, we'd probably notice by now:
         assert_equal(len(self.nodes[1].getrawmempool()), 0)
 
         # Verify accounting of mempool transactions after restart is correct
-        self.nodes[2].syncwithvalidationinterfacequeue()  # Flush mempool to wallet
+        #self.nodes[2].syncwithvalidationinterfacequeue()  # Flush mempool to wallet
         assert_equal(node2_balance, self.nodes[2].getbalance())
 
         self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
@@ -91,27 +84,29 @@ def run_test(self):
         self.start_node(0)
         wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5)
 
-        mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat')
-        mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat')
-        self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it")
-        os.remove(mempooldat0)
-        self.nodes[0].savemempool()
-        assert os.path.isfile(mempooldat0)
-
-        self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions")
-        os.rename(mempooldat0, mempooldat1)
-        self.stop_nodes()
-        self.start_node(1, extra_args=[])
-        wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5)
-
-        self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails")
-        # to test the exception we are setting bad permissions on a tmp file called mempool.dat.new
-        # which is an implementation detail that could change and break this test
-        mempooldotnew1 = mempooldat1 + '.new'
-        with os.fdopen(os.open(mempooldotnew1, os.O_CREAT, 0o000), 'w'):
-            pass
-        assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool)
-        os.remove(mempooldotnew1)
+        # Following code is ahead of our current repository state. Future back port.
+
+        # mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat')
+        # mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat')
+        # self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it")
+        # os.remove(mempooldat0)
+        # self.nodes[0].savemempool()
+        # assert os.path.isfile(mempooldat0)
+        #
+        # self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions")
+        # os.rename(mempooldat0, mempooldat1)
+        # self.stop_nodes()
+        # self.start_node(1, extra_args=[])
+        # wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5)
+        #
+        # self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails")
+        # # to test the exception we are setting bad permissions on a tmp file called mempool.dat.new
+        # # which is an implementation detail that could change and break this test
+        # mempooldotnew1 = mempooldat1 + '.new'
+        # with os.fdopen(os.open(mempooldotnew1, os.O_CREAT, 0o000), 'w'):
+        #     pass
+        # assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool)
+        # os.remove(mempooldotnew1)
 
 if __name__ == '__main__':
     MempoolPersistTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 5eef9e9cfdf1..96a19af45dca 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -88,6 +88,7 @@
     'feature_blockhashcache.py',                # ~ 100 sec
     'wallet_listtransactions.py',               # ~ 97 sec
     'mempool_reorg.py',                         # ~ 92 sec
+    'mempool_persist.py',
     'wallet_encryption.py',                     # ~ 89 sec
     'wallet_keypool.py',                        # ~ 88 sec
     'wallet_dump.py',                           # ~ 83 sec
@@ -117,7 +118,6 @@
     # 'mempool_limit.py', # We currently don't limit our mempool_reorg
     # 'interface_zmq.py',
     # 'rpc_getchaintips.py',
-    # 'mempool_persist.py',
     # 'rpc_users.py',
     # 'p2p_mempool.py',
     # 'mining_prioritisetransaction.py',

From 006c50341bc44bd9eb74a13a6a23556a8be49828 Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Sun, 17 Jan 2021 18:47:22 -0300
Subject: [PATCH 30/32] [wallet] fix zapwallettxes interaction with persistent
 mempool

zapwallettxes previously did not interact well with persistent mempool.
zapwallettxes would cause wallet transactions to be zapped, but they
would then be reloaded from the mempool on startup. This commit softsets
persistmempool to false if zapwallettxes is enabled so transactions are
actually zapped.
---
 src/init.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/init.cpp b/src/init.cpp
index 1ad82f29385c..e580e76a37d7 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -909,6 +909,11 @@ void InitParameterInteraction()
             LogPrintf("%s : parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__);
     }
 
+    // -zapwallettx implies dropping the mempool on startup
+    if (gArgs.GetBoolArg("-zapwallettxes", false) && gArgs.SoftSetBoolArg("-persistmempool", false)) {
+        LogPrintf("%s: parameter interaction: -zapwallettxes=<mode> -> setting -persistmempool=0\n", __func__);
+    }
+
     // -zapwallettx implies a rescan
     if (gArgs.GetBoolArg("-zapwallettxes", false)) {
         if (gArgs.SoftSetBoolArg("-rescan", true))

From d6d0ad9743a0453e8e7aa424fef9f3733ea8644f Mon Sep 17 00:00:00 2001
From: John Newbery <john@johnnewbery.com>
Date: Sun, 17 Jan 2021 18:50:21 -0300
Subject: [PATCH 31/32] [logs] fix zapwallettxes startup logs

---
 src/init.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index e580e76a37d7..cd98e6f568d8 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -909,15 +909,16 @@ void InitParameterInteraction()
             LogPrintf("%s : parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__);
     }
 
-    // -zapwallettx implies dropping the mempool on startup
-    if (gArgs.GetBoolArg("-zapwallettxes", false) && gArgs.SoftSetBoolArg("-persistmempool", false)) {
-        LogPrintf("%s: parameter interaction: -zapwallettxes=<mode> -> setting -persistmempool=0\n", __func__);
+    int zapwallettxes = gArgs.GetArg("-zapwallettxes", 0);
+    // -zapwallettxes implies dropping the mempool on startup
+    if (zapwallettxes != 0 && gArgs.SoftSetBoolArg("-persistmempool", false)) {
+        LogPrintf("%s: parameter interaction: -zapwallettxes=%s -> setting -persistmempool=0\n", __func__, zapwallettxes);
     }
 
-    // -zapwallettx implies a rescan
-    if (gArgs.GetBoolArg("-zapwallettxes", false)) {
+    // -zapwallettxes implies a rescan
+    if (zapwallettxes != 0) {
         if (gArgs.SoftSetBoolArg("-rescan", true))
-            LogPrintf("%s : parameter interaction: -zapwallettxes=<mode> -> setting -rescan=1\n", __func__);
+            LogPrintf("%s : parameter interaction: -zapwallettxes=%s -> setting -rescan=1\n", __func__, zapwallettxes);
     }
 }
 

From 9b9c61696b46435347d5271f0a9fa0e01e6909cb Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Mon, 18 Jan 2021 10:24:04 -0300
Subject: [PATCH 32/32] Fix missing zapwallettxes mode in wallet_hd.py
 functional test

---
 test/functional/wallet_hd.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py
index 7af4781c7789..5a56b9973a69 100755
--- a/test/functional/wallet_hd.py
+++ b/test/functional/wallet_hd.py
@@ -207,7 +207,7 @@ def run_test(self):
         assert_equal(z_add, z_add_3)
         # Restart, zap, and check balance: 1 PIV * (NUM_HD_ADDS + NUM_SHIELD_ADDS) recovered from seed
         self.stop_node(1)
-        self.start_node(1, extra_args=self.extra_args[1] + ['-zapwallettxes'])
+        self.start_node(1, extra_args=self.extra_args[1] + ['-zapwallettxes=1'])
         assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + NUM_SHIELD_ADDS)
 
 if __name__ == '__main__':