diff --git a/src/miner.h b/src/miner.h --- a/src/miner.h +++ b/src/miner.h @@ -53,6 +53,12 @@ nSigOpCountWithAncestors = entry->GetSigOpCountWithAncestors(); } + Amount GetModifiedFee() const { return iter->GetModifiedFee(); } + uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + Amount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } + size_t GetTxSize() const { return iter->GetTxSize(); } + const CTransaction &GetTx() const { return iter->GetTx(); } + CTxMemPool::txiter iter; uint64_t nSizeWithAncestors; Amount nModFeesWithAncestors; @@ -79,21 +85,6 @@ } }; -// This matches the calculation in CompareTxMemPoolEntryByAncestorFee, -// except operating on CTxMemPoolModifiedEntry. -// TODO: refactor to avoid duplication of this logic. -struct CompareModifiedEntry { - bool operator()(const CTxMemPoolModifiedEntry &a, - const CTxMemPoolModifiedEntry &b) const { - double f1 = b.nSizeWithAncestors * (a.nModFeesWithAncestors / SATOSHI); - double f2 = a.nSizeWithAncestors * (b.nModFeesWithAncestors / SATOSHI); - if (f1 == f2) { - return CTxMemPool::CompareIteratorByHash()(a.iter, b.iter); - } - return f1 > f2; - } -}; - // A comparator that sorts transactions based on number of ancestors. // This is sufficient to sort an ancestor package in an order that is valid // to appear in a block. @@ -117,7 +108,7 @@ // Reuse same tag from CTxMemPool's similar index boost::multi_index::tag, boost::multi_index::identity, - CompareModifiedEntry>>> + CompareTxMemPoolEntryByAncestorFee>>> indexed_modified_transaction_set; typedef indexed_modified_transaction_set::nth_index<0>::type::iterator diff --git a/src/miner.cpp b/src/miner.cpp --- a/src/miner.cpp +++ b/src/miner.cpp @@ -495,7 +495,8 @@ // Try to compare the mapTx entry to the mapModifiedTx entry. iter = mempool->mapTx.project<0>(mi); if (modit != mapModifiedTx.get().end() && - CompareModifiedEntry()(*modit, CTxMemPoolModifiedEntry(iter))) { + CompareTxMemPoolEntryByAncestorFee()( + *modit, CTxMemPoolModifiedEntry(iter))) { // The best entry in mapModifiedTx has higher score than the one // from mapTx. Switch which transaction (package) to consider iter = modit->iter; diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -578,6 +578,25 @@ sortedOrder.insert(sortedOrder.begin(), tx7.GetId().ToString()); CheckSort(pool, sortedOrder, "MempoolAncestorIndexingTest4"); + + // High-fee parent, low-fee child + // tx7 -> tx8 + CMutableTransaction tx8 = CMutableTransaction(); + tx8.vin.resize(1); + tx8.vin[0].prevout = COutPoint(tx7.GetId(), 0); + tx8.vin[0].scriptSig = CScript() << OP_11; + tx8.vout.resize(1); + tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx8.vout[0].nValue = 10 * COIN; + + // Check that we sort by min(feerate, ancestor_feerate): + // set the fee so that the ancestor feerate is above tx1/5, + // but the transaction's own feerate is lower + pool.addUnchecked(tx8.GetId(), + entry.Fee(Amount(5000 * SATOSHI)).FromTx(tx8)); + sortedOrder.insert(sortedOrder.end() - 1, tx8.GetId().ToString()); + CheckSort(pool, sortedOrder, + "MempoolAncestorIndexingTest5"); } BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -243,24 +243,14 @@ class CompareTxMemPoolEntryByDescendantScore { public: bool operator()(const CTxMemPoolEntry &a, const CTxMemPoolEntry &b) const { - bool fUseADescendants = UseDescendantScore(a); - bool fUseBDescendants = UseDescendantScore(b); + double a_mod_fee, a_size, b_mod_fee, b_size; - double aModFee = (fUseADescendants ? a.GetModFeesWithDescendants() - : a.GetModifiedFee()) / - SATOSHI; - double aSize = - fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize(); - - double bModFee = (fUseBDescendants ? b.GetModFeesWithDescendants() - : b.GetModifiedFee()) / - SATOSHI; - double bSize = - fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize(); + GetModFeeAndSize(a, a_mod_fee, a_size); + GetModFeeAndSize(b, b_mod_fee, b_size); // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). - double f1 = aModFee * bSize; - double f2 = aSize * bModFee; + double f1 = a_mod_fee * b_size; + double f2 = a_size * b_mod_fee; if (f1 == f2) { return a.GetTime() >= b.GetTime(); @@ -268,11 +258,21 @@ return f1 < f2; } - // Calculate which score to use for an entry (avoiding division). - bool UseDescendantScore(const CTxMemPoolEntry &a) const { + // Return the fee/size we're using for sorting this entry. + void GetModFeeAndSize(const CTxMemPoolEntry &a, double &mod_fee, + double &size) const { + // Compare feerate with descendants to feerate of the transaction, and + // return the fee/size for the max. double f1 = a.GetSizeWithDescendants() * (a.GetModifiedFee() / SATOSHI); double f2 = a.GetTxSize() * (a.GetModFeesWithDescendants() / SATOSHI); - return f2 > f1; + + if (f2 > f1) { + mod_fee = a.GetModFeesWithDescendants() / SATOSHI; + size = a.GetSizeWithDescendants(); + } else { + mod_fee = a.GetModifiedFee() / SATOSHI; + size = a.GetTxSize(); + } } }; @@ -299,25 +299,45 @@ } }; +/** \class CompareTxMemPoolEntryByAncestorScore + * + * Sort an entry by min(score/size of entry's tx, score/size with all + * ancestors). + */ class CompareTxMemPoolEntryByAncestorFee { public: - bool operator()(const CTxMemPoolEntry &a, const CTxMemPoolEntry &b) const { - double aFees = a.GetModFeesWithAncestors() / SATOSHI; - double aSize = a.GetSizeWithAncestors(); + template bool operator()(const T &a, const T &b) const { + double a_mod_fee, a_size, b_mod_fee, b_size; - double bFees = b.GetModFeesWithAncestors() / SATOSHI; - double bSize = b.GetSizeWithAncestors(); + GetModFeeAndSize(a, a_mod_fee, a_size); + GetModFeeAndSize(b, b_mod_fee, b_size); // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). - double f1 = aFees * bSize; - double f2 = aSize * bFees; + double f1 = a_mod_fee * b_size; + double f2 = a_size * b_mod_fee; if (f1 == f2) { return a.GetTx().GetId() < b.GetTx().GetId(); } - return f1 > f2; } + + // Return the fee/size we're using for sorting this entry. + template + void GetModFeeAndSize(const T &a, double &mod_fee, double &size) const { + // Compare feerate with ancestors to feerate of the transaction, and + // return the fee/size for the min. + double f1 = a.GetSizeWithAncestors() * (a.GetModifiedFee() / SATOSHI); + double f2 = a.GetTxSize() * (a.GetModFeesWithAncestors() / SATOSHI); + + if (f1 > f2) { + mod_fee = a.GetModFeesWithAncestors() / SATOSHI; + size = a.GetSizeWithAncestors(); + } else { + mod_fee = a.GetModifiedFee() / SATOSHI; + size = a.GetTxSize(); + } + } }; // Multi_index tag names