diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 37778322151..f60ab27b187 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5425,8 +5425,8 @@ public: bool operator()(std::set::iterator a, std::set::iterator b) { /* As std::make_heap produces a max-heap, we want the entries with the - * fewest ancestors/highest fee to sort later. */ - return m_mempool->CompareDepthAndScore(*b, *a); + * higher mining score to sort later. */ + return m_mempool->CompareMiningScoreWithTopology(*b, *a); } }; } // namespace diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f2a63c26d26..fa7da083a14 100644 --- a/src/txmempool.cpp +++ b/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: - * a is not in the mempool, but b is - * both are in the mempool and a has fewer ancestors than b - * both are in the mempool and a has a higher score than b + /* Return `true` if hasha should be considered sooner than hashb, namely when: + * a is not in the mempool but b is, or + * both are in the mempool but a is sorted before b in the total mempool ordering + * (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(); - if (counta == countb) { - return CompareTxMemPoolEntryByScore()(*i.value(), *j.value()); - } - return counta < countb; + + return m_txgraph->CompareMainOrder(*i.value(), *j.value()) < 0; } namespace { diff --git a/src/txmempool.h b/src/txmempool.h index 5834cd0065a..232538aec36 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -379,7 +379,7 @@ public: void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& 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); diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index b5f994e8030..8f53d913c96 100755 --- a/test/functional/mempool_packages.py +++ b/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 - # of node0 to be based on ancestor count (so that the first 10 - # descendants of parent_transaction relay before the later ones). - for tx in chain[:CUSTOM_DESCENDANT_LIMIT-1]: - assert tx in mempool1 - for tx in chain[CUSTOM_DESCENDANT_LIMIT:]: - assert tx not in mempool1 - for tx in mempool1: - entry0 = self.nodes[0].getmempoolentry(tx) - entry1 = self.nodes[1].getmempoolentry(tx) - 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']) + for tx in chain: + if tx in mempool1: + entry0 = self.nodes[0].getmempoolentry(tx) + entry1 = self.nodes[1].getmempoolentry(tx) + assert not entry0['unbroadcast'] + assert not entry1['unbroadcast'] + assert entry1["descendantcount"] <= CUSTOM_DESCENDANT_LIMIT + 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)