Don't rely on a finite upper bound on fee rate or priority.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

src/policy/fees.cpp

@@ -16,10 +16,14 @@ void TxConfirmStats::Initialize(std::vector<double>& defaultBuckets,
 {
     decay = _decay;
     dataTypeString = _dataTypeString;
-    for (unsigned int i = 0; i < defaultBuckets.size(); i++) {
+
-        buckets.push_back(defaultBuckets[i]);
+    buckets.insert(buckets.end(), defaultBuckets.begin(), defaultBuckets.end());
-        bucketMap[defaultBuckets[i]] = i;
+    buckets.push_back(std::numeric_limits<double>::infinity());
+
+    for (unsigned int i = 0; i < buckets.size(); i++) {
+        bucketMap[buckets[i]] = i;
     }
+
     confAvg.resize(maxConfirms);
     curBlockConf.resize(maxConfirms);
     unconfTxs.resize(maxConfirms);
@@ -49,13 +53,19 @@ void TxConfirmStats::ClearCurrent(unsigned int nBlockHeight)
     }
 }
 
+unsigned int TxConfirmStats::FindBucketIndex(double val)
+{
+    auto it = bucketMap.lower_bound(val);
+    assert(it != bucketMap.end());
+    return it->second;
+}
 
 void TxConfirmStats::Record(int blocksToConfirm, double val)
 {
     // blocksToConfirm is 1-based
     if (blocksToConfirm < 1)
         return;
-    unsigned int bucketindex = bucketMap.lower_bound(val)->second;
+    unsigned int bucketindex = FindBucketIndex(val);
     for (size_t i = blocksToConfirm; i <= curBlockConf.size(); i++) {
         curBlockConf[i - 1][bucketindex]++;
     }
@@ -246,7 +256,7 @@ void TxConfirmStats::Read(CAutoFile& filein)
 
 unsigned int TxConfirmStats::NewTx(unsigned int nBlockHeight, double val)
 {
-    unsigned int bucketindex = bucketMap.lower_bound(val)->second;
+    unsigned int bucketindex = FindBucketIndex(val);
     unsigned int blockIndex = nBlockHeight % unconfTxs.size();
     unconfTxs[blockIndex][bucketindex]++;
     LogPrint("estimatefee", "adding to %s", dataTypeString);
@@ -306,7 +316,6 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee)
     for (double bucketBoundary = minTrackedFee.GetFeePerK(); bucketBoundary <= MAX_FEERATE; bucketBoundary *= FEE_SPACING) {
         vfeelist.push_back(bucketBoundary);
     }
-    vfeelist.push_back(INF_FEERATE);
     feeStats.Initialize(vfeelist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY, "FeeRate");
 
     minTrackedPriority = AllowFreeThreshold() < MIN_PRIORITY ? MIN_PRIORITY : AllowFreeThreshold();
@@ -314,7 +323,6 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const CFeeRate& _minRelayFee)
     for (double bucketBoundary = minTrackedPriority; bucketBoundary <= MAX_PRIORITY; bucketBoundary *= PRI_SPACING) {
         vprilist.push_back(bucketBoundary);
     }
-    vprilist.push_back(INF_PRIORITY);
     priStats.Initialize(vprilist, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY, "Priority");
 
     feeUnlikely = CFeeRate(0);

src/policy/fees.h

@@ -73,7 +73,9 @@ static const double DEFAULT_DECAY = .998;
  * We will instantiate two instances of this class, one to track transactions
  * that were included in a block due to fee, and one for tx's included due to
  * priority.  We will lump transactions into a bucket according to their approximate
- * fee or priority and then track how long it took for those txs to be included in a block
+ * fee or priority and then track how long it took for those txs to be included
+ * in a block. There is always a bucket into which any given double value
+ * (representing a fee or priority) falls.
  *
  * The tracking of unconfirmed (mempool) transactions is completely independent of the
  * historical tracking of transactions that have been confirmed in a block.
@@ -118,9 +120,14 @@ private:
     std::vector<int> oldUnconfTxs;
 
 public:
+    /** Find the bucket index of a given value */
+    unsigned int FindBucketIndex(double val);
+
     /**
      * Initialize the data structures.  This is called by BlockPolicyEstimator's
-     * constructor with default values.
+     * constructor with default values.  A final bucket is created implicitly for
+     * values greater than the last upper limit in defaultBuckets.
+     *
      * @param defaultBuckets contains the upper limits for the bucket boundaries
      * @param maxConfirms max number of confirms to track
      * @param decay how much to decay the historical moving average per block

src/test/policyestimator_tests.cpp

@@ -183,4 +183,23 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     }
 }
 
+
+BOOST_AUTO_TEST_CASE(TxConfirmStats_FindBucketIndex)
+{
+    std::vector<double> buckets {0.0, 3.5, 42.0};
+    TxConfirmStats txcs;
+
+    txcs.Initialize(buckets, MAX_BLOCK_CONFIRMS, DEFAULT_DECAY, "Test");
+
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(-1.0), 0);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(0.0), 0);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(1.0), 1);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(3.5), 1);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(4.0), 2);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(43.0), 3);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(INF_FEERATE), 3);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(2.0*INF_FEERATE), 3);
+    BOOST_CHECK_EQUAL(txcs.FindBucketIndex(nan("")), 0);
+}
+
 BOOST_AUTO_TEST_SUITE_END()