Merge pull request dogecoin#3389 from patricklodder/1.14.7-rework-smartfee-params

policy: tune feerate estimator parameters

Author: chromatic

src/policy/fees.h

@@ -169,26 +169,70 @@ class TxConfirmStats
 /** Track confirm delays up to 25 blocks, can't estimate beyond that */
 static const unsigned int MAX_BLOCK_CONFIRMS = 25;
 
-/** Decay of .998 is a half-life of 346 blocks or about 2.4 days */
-static const double DEFAULT_DECAY = .998;
+/** Inverse rate of decay of statistical history
+ * Reduces all historical block stats by this multiplier, for each new block
+ *
+ * A value of 0.98845 corresponds to a half-life of 60 blocks or about 1 hour
+ *
+ * The calculation involves the rate constant for first order reactions, "ln 2",
+ * which is approximately 0.693: 1 - ( ln(2) / <t0.5> )
+ *
+ * Rationale: The mempool from hours ago should have only a light effect
+ * on estimates, to better deal with volatility.
+ */
+static const double DEFAULT_DECAY = 1.0 - (0.693 / 60.0); // 0.98845
+
+/** Historical confidence level
+ * Determines validity of results based on the historical inclusion rate of
+ * transactions of this feerate
+ *
+ * Require greater than 80% of X feerate transactions to be confirmed within
+ * Y blocks for X to be big enough
+ *
+ * Rationale: More conservative than Bitcoin Core's 60% for "half" confidence,
+ * but less conservative than the 85% for "normal" confidence, because the
+ * Dogecoin miners have a hightened incentive to not orphan blocks while the
+ * impact of not being included in a block is only ~1/10th of Bitcoin's when
+ * expressed in time expired.
+ */
+static const double MIN_SUCCESS_PCT = .8;
 
-/** Require greater than 95% of X feerate transactions to be confirmed within Y blocks for X to be big enough */
-static const double MIN_SUCCESS_PCT = .95;
+/** Bucket significance in transactions per bucket
+ * Filters which ranges of feerates are significant enough to consider
+ *
+ * Require an avg of 1 tx in the combined feerate bucket per 10 blocks to have
+ * stat significance
+ *
+ * Rationale: Reduced this from 1 tx per feerate per block as lower values make
+ * more buckets relevant during periods of low transaction volume, uniform fee
+ * transaction surges (i.e. spam) or miners rapidly finding successive blocks
+ * while there are no transactions matching their feefilter
+ *
+ * Copied from Bitcoin Core 0.15-25.0
+ */
+static const double SUFFICIENT_FEETXS = 0.1;
 
-/** Require an avg of 1 tx in the combined feerate bucket per block to have stat significance */
-static const double SUFFICIENT_FEETXS = 1;
+// Minimum and Maximum values for tracking feerates, in koinu per kB
+static constexpr double MIN_FEERATE = COIN / 1000.0;  //!< 100,000 - equals 100 koinu per byte
+static const double MAX_FEERATE = COIN * 10.0;    //!< 1000,000,000 koinu - equals 10 DOGE/kb
 
-// Minimum and Maximum values for tracking feerates
-static constexpr double MIN_FEERATE = 10;
-static const double MAX_FEERATE = 1e7;
+// Feerate and priority for the upper border of the highest bucket, in koinu per kB
 static const double INF_FEERATE = MAX_MONEY;
 static const double INF_PRIORITY = 1e9 * MAX_MONEY;
 
-// We have to lump transactions into buckets based on feerate, but we want to be able
-// to give accurate estimates over a large range of potential feerates
-// Therefore it makes sense to exponentially space the buckets
-/** Spacing of FeeRate buckets */
-static const double FEE_SPACING = 1.1;
+/** Spacing of FeeRate buckets
+ * Determines the (exponential) granularity of the fee buckets
+ *
+ * We have to lump transactions into buckets based on feerate, but we want to be
+ * able to give accurate estimates over a large range of potential feerates
+ * Therefore it makes sense to exponentially space the buckets
+ *
+ * Rationale: previously, spacing was 1.1; we reduce this to 1.05 to have finer
+ * grained buckets
+ *
+ * Copied from Bitcoin Core 0.15-25.0
+ */
+static const double FEE_SPACING = 1.05;
 
 /**
  *  We want to be able to estimate feerates that are needed on tx's to be included in

src/test/policyestimator_tests.cpp

@@ -16,10 +16,10 @@ BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
 {
-    CTxMemPool mpool(CFeeRate(1000));
+    CTxMemPool mpool(CFeeRate(100000));
     TestMemPoolEntryHelper entry;
-    CAmount basefee(2000);
-    CAmount deltaFee(100);
+    CAmount basefee(200000);
+    CAmount deltaFee(100); // error margin for feerate tests
     std::vector<CAmount> feeV;
 
     // Populate vectors of increasing fees
@@ -49,8 +49,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     int blocknum = 0;
 
     // Loop through 200 blocks
-    // At a decay .998 and 4 fee transactions per block
-    // This makes the tx count about 1.33 per bucket, above the 1 threshold
+    // At a decay .98845 and 4 fee transactions per block
+    // This makes the tx count about 0.39 per bucket, above the 0.1 threshold
     while (blocknum < 200) {
         for (int j = 0; j < 10; j++) { // For each fee
             for (int k = 0; k < 4; k++) { // add 4 fee txs
@@ -74,20 +74,12 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         }
         mpool.removeForBlock(block, ++blocknum);
         block.clear();
-        if (blocknum == 30) {
-            // At this point we should need to combine 5 buckets to get enough data points
-            // So estimateFee(1,2,3) should fail and estimateFee(4) should return somewhere around
-            // 8*baserate.  estimateFee(4) %'s are 100,100,100,100,90 = average 98%
-            BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0));
-            BOOST_CHECK(mpool.estimateFee(2) == CFeeRate(0));
-            BOOST_CHECK(mpool.estimateFee(3) == CFeeRate(0));
-            BOOST_CHECK(mpool.estimateFee(4).GetFeePerK() < 8*baseRate.GetFeePerK() + deltaFee);
-            BOOST_CHECK(mpool.estimateFee(4).GetFeePerK() > 8*baseRate.GetFeePerK() - deltaFee);
-            int answerFound;
-            BOOST_CHECK(mpool.estimateSmartFee(1, &answerFound) == mpool.estimateFee(4) && answerFound == 4);
-            BOOST_CHECK(mpool.estimateSmartFee(3, &answerFound) == mpool.estimateFee(4) && answerFound == 4);
-            BOOST_CHECK(mpool.estimateSmartFee(4, &answerFound) == mpool.estimateFee(4) && answerFound == 4);
-            BOOST_CHECK(mpool.estimateSmartFee(8, &answerFound) == mpool.estimateFee(8) && answerFound == 8);
+        if (blocknum == 3) {
+            // At this point we should need to combine 2 buckets to get enough data points
+            // So estimateFee(2) should return somewhere around 9*baserate.
+            // estimateFee(2) %'s are 100,100,90 = average 97%
+            BOOST_CHECK(mpool.estimateFee(2).GetFeePerK() < 9*baseRate.GetFeePerK() + deltaFee);
+            BOOST_CHECK(mpool.estimateFee(2).GetFeePerK() > 9*baseRate.GetFeePerK() - deltaFee);
         }
     }
 
@@ -97,14 +89,14 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     // third highest feerate is 8*base rate, and gets in 8/10 blocks = 80%,
     // so estimateFee(1) would return 10*baseRate but is hardcoded to return failure
     // Second highest feerate has 100% chance of being included by 2 blocks,
-    // so estimateFee(2) should return 9*baseRate etc...
-    for (int i = 1; i < 10;i++) {
+    // thus at 80% confidence, estimateFee(2) should return 8*baseRate etc...
+    for (int i = 1; i < 8;i++) {
         origFeeEst.push_back(mpool.estimateFee(i).GetFeePerK());
         if (i > 2) { // Fee estimates should be monotonically decreasing
             BOOST_CHECK(origFeeEst[i-1] <= origFeeEst[i-2]);
         }
-        int mult = 11-i;
-        if (i > 1) {
+        int mult = 9-i;
+        if (i > 1 && i < 8) {
             BOOST_CHECK(origFeeEst[i-1] < mult*baseRate.GetFeePerK() + deltaFee);
             BOOST_CHECK(origFeeEst[i-1] > mult*baseRate.GetFeePerK() - deltaFee);
         }
@@ -119,7 +111,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         mpool.removeForBlock(block, ++blocknum);
 
     BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0));
-    for (int i = 2; i < 10;i++) {
+    for (int i = 2; i < 8;i++) {
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee);
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
     }
@@ -140,7 +132,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     }
 
     int answerFound;
-    for (int i = 1; i < 10;i++) {
+    for (int i = 1; i < 8;i++) {
         BOOST_CHECK(mpool.estimateFee(i) == CFeeRate(0) || mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
         BOOST_CHECK(mpool.estimateSmartFee(i, &answerFound).GetFeePerK() > origFeeEst[answerFound-1] - deltaFee);
     }
@@ -158,7 +150,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
     mpool.removeForBlock(block, 265);
     block.clear();
     BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0));
-    for (int i = 2; i < 10;i++) {
+    for (int i = 2; i < 8;i++) {
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
     }
 
@@ -180,7 +172,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
         block.clear();
     }
     BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0));
-    for (int i = 2; i < 10; i++) {
+    for (int i = 2; i < 8; i++) {
         BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] - deltaFee);
     }