From dbb9e3699b8e835fd72a5db2c22927d828484c32 Mon Sep 17 00:00:00 2001
From: Alex Morcos <morcos@chaincode.com>
Date: Wed, 15 Feb 2017 09:24:11 -0500
Subject: [PATCH] Give CBlockPolicyEstimator it's own lock

---
 src/policy/fees.cpp | 32 ++++++++++++++++++++++----------
 src/policy/fees.h   |  3 +++
 src/txmempool.cpp   |  4 ----
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index 38e07dc345..05f2a20e9a 100644
--- a/src/policy/fees.cpp
+++ b/src/policy/fees.cpp
@@ -290,6 +290,7 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe
 // of no harm to try to remove them again.
 bool CBlockPolicyEstimator::removeTx(uint256 hash)
 {
+    LOCK(cs_feeEstimator);
     std::map<uint256, TxStatsInfo>::iterator pos = mapMemPoolTxs.find(hash);
     if (pos != mapMemPoolTxs.end()) {
         feeStats.removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex);
@@ -315,6 +316,7 @@ CBlockPolicyEstimator::CBlockPolicyEstimator()
 
 void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate)
 {
+    LOCK(cs_feeEstimator);
     unsigned int txHeight = entry.GetHeight();
     uint256 hash = entry.GetTx().GetHash();
     if (mapMemPoolTxs.count(hash)) {
@@ -374,6 +376,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
 void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
                                          std::vector<const CTxMemPoolEntry*>& entries)
 {
+    LOCK(cs_feeEstimator);
     if (nBlockHeight <= nBestSeenHeight) {
         // Ignore side chains and re-orgs; assuming they are random
         // they don't affect the estimate.
@@ -410,6 +413,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
 
 CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget)
 {
+    LOCK(cs_feeEstimator);
     // Return failure if trying to analyze a target we're not tracking
     // It's not possible to get reasonable estimates for confTarget of 1
     if (confTarget <= 1 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
@@ -427,18 +431,24 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun
 {
     if (answerFoundAtTarget)
         *answerFoundAtTarget = confTarget;
-    // Return failure if trying to analyze a target we're not tracking
-    if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
-        return CFeeRate(0);
-
-    // It's not possible to get reasonable estimates for confTarget of 1
-    if (confTarget == 1)
-        confTarget = 2;
 
     double median = -1;
-    while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) {
-        median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight);
-    }
+
+    {
+        LOCK(cs_feeEstimator);
+
+        // Return failure if trying to analyze a target we're not tracking
+        if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
+            return CFeeRate(0);
+
+        // It's not possible to get reasonable estimates for confTarget of 1
+        if (confTarget == 1)
+            confTarget = 2;
+
+        while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) {
+            median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight);
+        }
+    } // Must unlock cs_feeEstimator before taking mempool locks
 
     if (answerFoundAtTarget)
         *answerFoundAtTarget = confTarget - 1;
@@ -456,12 +466,14 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun
 
 void CBlockPolicyEstimator::Write(CAutoFile& fileout)
 {
+    LOCK(cs_feeEstimator);
     fileout << nBestSeenHeight;
     feeStats.Write(fileout);
 }
 
 void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion)
 {
+    LOCK(cs_feeEstimator);
     int nFileBestSeenHeight;
     filein >> nFileBestSeenHeight;
     feeStats.Read(filein);
diff --git a/src/policy/fees.h b/src/policy/fees.h
index 7abd1d43a1..e11a6582a4 100644
--- a/src/policy/fees.h
+++ b/src/policy/fees.h
@@ -8,6 +8,7 @@
 #include "amount.h"
 #include "uint256.h"
 #include "random.h"
+#include "sync.h"
 
 #include <map>
 #include <string>
@@ -249,6 +250,8 @@ private:
     unsigned int trackedTxs;
     unsigned int untrackedTxs;
 
+    mutable CCriticalSection cs_feeEstimator;
+
     /** Process a transaction confirmed in a block*/
     bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry);
 
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 3dc7c11f2d..7fac48d3c5 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -845,12 +845,10 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
 
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
 {
-    LOCK(cs);
     return minerPolicyEstimator->estimateFee(nBlocks);
 }
 CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const
 {
-    LOCK(cs);
     return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this);
 }
 
@@ -858,7 +856,6 @@ bool
 CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const
 {
     try {
-        LOCK(cs);
         fileout << 139900; // version required to read: 0.13.99 or later
         fileout << CLIENT_VERSION; // version that wrote the file
         minerPolicyEstimator->Write(fileout);
@@ -878,7 +875,6 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein)
         filein >> nVersionRequired >> nVersionThatWrote;
         if (nVersionRequired > CLIENT_VERSION)
             return error("CTxMemPool::ReadFeeEstimates(): up-version (%d) fee estimate file", nVersionRequired);
-        LOCK(cs);
         minerPolicyEstimator->Read(filein, nVersionThatWrote);
     }
     catch (const std::exception&) {