From 0a22a52918ad5af6d105b4f5ae9dd6c52199f0e8 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@gmail.com>
Date: Sat, 13 Jan 2018 15:57:30 -0500
Subject: [PATCH] Use mempool's ancestor sort in transaction selection

Transaction selection for mining tracks ancestor feerates that are
modified based on transactions that have already been selected.  This
commit de-duplicates the code so that the ancestor feerate sorting used
by the mempool can also be directly applied to the miner.
---
 src/miner.cpp   |  2 +-
 src/miner.h     | 23 +++++++----------------
 src/txmempool.h |  6 ++++--
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/src/miner.cpp b/src/miner.cpp
index 4e63ab4df05..dda52790c61 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -352,7 +352,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
             // Try to compare the mapTx entry to the mapModifiedTx entry
             iter = mempool.mapTx.project<0>(mi);
             if (modit != mapModifiedTx.get<ancestor_score>().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
diff --git a/src/miner.h b/src/miner.h
index 698b4a47888..9c086332d49 100644
--- a/src/miner.h
+++ b/src/miner.h
@@ -41,6 +41,12 @@ struct CTxMemPoolModifiedEntry {
         nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors();
     }
 
+    int64_t GetModifiedFee() const { return iter->GetModifiedFee(); }
+    uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
+    CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
+    size_t GetTxSize() const { return iter->GetTxSize(); }
+    const CTransaction& GetTx() const { return iter->GetTx(); }
+
     CTxMemPool::txiter iter;
     uint64_t nSizeWithAncestors;
     CAmount nModFeesWithAncestors;
@@ -67,21 +73,6 @@ struct modifiedentry_iter {
     }
 };
 
-// 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 = (double)a.nModFeesWithAncestors * b.nSizeWithAncestors;
-        double f2 = (double)b.nModFeesWithAncestors * a.nSizeWithAncestors;
-        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.
@@ -106,7 +97,7 @@ typedef boost::multi_index_container<
             // Reuse same tag from CTxMemPool's similar index
             boost::multi_index::tag<ancestor_score>,
             boost::multi_index::identity<CTxMemPoolModifiedEntry>,
-            CompareModifiedEntry
+            CompareTxMemPoolEntryByAncestorFee
         >
     >
 > indexed_modified_transaction_set;
diff --git a/src/txmempool.h b/src/txmempool.h
index a5eba9787ec..d32131d7a80 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -273,7 +273,8 @@ public:
 class CompareTxMemPoolEntryByAncestorFee
 {
 public:
-    bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
+    template<typename T>
+    bool operator()(const T& a, const T& b) const
     {
         double a_mod_fee, a_size, b_mod_fee, b_size;
 
@@ -291,7 +292,8 @@ public:
     }
 
     // Return the fee/size we're using for sorting this entry.
-    void GetModFeeAndSize(const CTxMemPoolEntry &a, double &mod_fee, double &size) const
+    template <typename T>
+    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.