Use cluster linearization for transaction relay sort order

Previously, transaction batches were first sorted by ancestor count and then
feerate, to ensure transactions are announced in a topologically valid order,
while prioritizing higher feerate transactions. Ancestor count is a crude
topological sort criteria, so replace this with linearization order so that the
highest feerate transactions (as would be observed by the mining algorithm) are
relayed before lower feerate ones, in a topologically valid way.

This also fixes a test that only worked due to the ancestor-count-based sort
order.

src/net_processing.cpp

@@ -5425,8 +5425,8 @@ public:
     bool operator()(std::set<Wtxid>::iterator a, std::set<Wtxid>::iterator b)
     {
         /* As std::make_heap produces a max-heap, we want the entries with the
-         * fewest ancestors/highest fee to sort later. */
+         * higher mining score to sort later. */
-        return m_mempool->CompareDepthAndScore(*b, *a);
+        return m_mempool->CompareMiningScoreWithTopology(*b, *a);
     }
 };
 } // namespace

src/txmempool.cpp

@@ -803,24 +803,20 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
     assert(innerUsage == cachedInnerUsage);
 }
 
-bool CTxMemPool::CompareDepthAndScore(const Wtxid& hasha, const Wtxid& hashb) const
+bool CTxMemPool::CompareMiningScoreWithTopology(const Wtxid& hasha, const Wtxid& hashb) const
 {
-    /* Return `true` if hasha should be considered sooner than hashb. Namely when:
+    /* Return `true` if hasha should be considered sooner than hashb, namely when:
-     *   a is not in the mempool, but b is
+     *     a is not in the mempool but b is, or
-     *   both are in the mempool and a has fewer ancestors than b
+     *     both are in the mempool but a is sorted before b in the total mempool ordering
-     *   both are in the mempool and a has a higher score than b
+     *     (which takes dependencies and (chunk) feerates into account).
      */
     LOCK(cs);
     auto j{GetIter(hashb)};
     if (!j.has_value()) return false;
     auto i{GetIter(hasha)};
     if (!i.has_value()) return true;
-    uint64_t counta = i.value()->GetCountWithAncestors();
+
-    uint64_t countb = j.value()->GetCountWithAncestors();
+    return m_txgraph->CompareMainOrder(*i.value(), *j.value()) < 0;
-    if (counta == countb) {
-        return CompareTxMemPoolEntryByScore()(*i.value(), *j.value());
-    }
-    return counta < countb;
 }
 
 namespace {

src/txmempool.h

@@ -379,7 +379,7 @@ public:
     void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
     void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    bool CompareDepthAndScore(const Wtxid& hasha, const Wtxid& hashb) const;
+    bool CompareMiningScoreWithTopology(const Wtxid& hasha, const Wtxid& hashb) const;
     bool isSpent(const COutPoint& outpoint) const;
     unsigned int GetTransactionsUpdated() const;
     void AddTransactionsUpdated(unsigned int n);

test/functional/mempool_packages.py

@@ -245,21 +245,17 @@ class MempoolPackagesTest(BitcoinTestFramework):
         mempool1 = self.nodes[1].getrawmempool(False)
         assert set(mempool1).issubset(set(mempool0))
         assert parent_transaction in mempool1
-        # Note: this test is brittle, because it relies on the relay sort order
+        for tx in chain:
-        # of node0 to be based on ancestor count (so that the first 10
+            if tx in mempool1:
-        # descendants of parent_transaction relay before the later ones).
+                entry0 = self.nodes[0].getmempoolentry(tx)
-        for tx in chain[:CUSTOM_DESCENDANT_LIMIT-1]:
+                entry1 = self.nodes[1].getmempoolentry(tx)
-            assert tx in mempool1
+                assert not entry0['unbroadcast']
-        for tx in chain[CUSTOM_DESCENDANT_LIMIT:]:
+                assert not entry1['unbroadcast']
-            assert tx not in mempool1
+                assert entry1["descendantcount"] <= CUSTOM_DESCENDANT_LIMIT
-        for tx in mempool1:
+                assert_equal(entry1['fees']['base'], entry0['fees']['base'])
-            entry0 = self.nodes[0].getmempoolentry(tx)
+                assert_equal(entry1['vsize'], entry0['vsize'])
-            entry1 = self.nodes[1].getmempoolentry(tx)
+                assert_equal(entry1['depends'], entry0['depends'])
-            assert not entry0['unbroadcast']
+
-            assert not entry1['unbroadcast']
-            assert_equal(entry1['fees']['base'], entry0['fees']['base'])
-            assert_equal(entry1['vsize'], entry0['vsize'])
-            assert_equal(entry1['depends'], entry0['depends'])
         # Test reorg handling
         # First, the basics:
         self.generate(self.nodes[0], 1)