fees: move StringForBlockPolicyEstimateReason to block policy estimator

Now that the wallet reports its own FeeReason, StringForBlockPolicyEstimateReason
is only used internally by the block policy estimator. Move it from
common/messages into the block policy fee rate estimator.

Also add the detailed FeeCalculation debug log to estimateSmartFee, where
the FeeCalculation data originates, and always populate feeCalc locally so
the log is available even when the caller does not pass a valid
FeeCalculation pointer.

src/common/messages.cpp

@@ -7,7 +7,6 @@
 
 #include <common/types.h>
 #include <node/types.h>
-#include <policy/fees/block_policy_estimator.h>
 #include <tinyformat.h>
 #include <util/check.h>
 #include <util/fees.h>
@@ -41,23 +40,6 @@ std::string StringForFeeReason(FeeReason reason)
     return reason_string->second;
 }
 
-std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason)
-{
-    switch (reason) {
-    case BlockPolicyEstimateReason::NONE:
-        return "None";
-    case BlockPolicyEstimateReason::HALF_ESTIMATE:
-        return "Half Target 60% Threshold";
-    case BlockPolicyEstimateReason::FULL_ESTIMATE:
-        return "Target 85% Threshold";
-    case BlockPolicyEstimateReason::DOUBLE_ESTIMATE:
-        return "Double Target 95% Threshold";
-    case BlockPolicyEstimateReason::CONSERVATIVE:
-        return "Conservative Double Target longer horizon";
-    } // no default case, so the compiler can warn about missing cases
-    assert(false);
-}
-
 const std::vector<std::pair<std::string, FeeEstimateMode>>& FeeModeMap()
 {
     static const std::vector<std::pair<std::string, FeeEstimateMode>> FEE_MODES = {

src/common/messages.h

@@ -18,7 +18,6 @@
 struct bilingual_str;
 enum class FeeEstimateMode;
 enum class FeeReason;
-enum class BlockPolicyEstimateReason;
 namespace node {
 enum class TransactionError;
 } // namespace node
@@ -28,7 +27,6 @@ enum class PSBTError;
 
 bool FeeModeFromString(std::string_view mode_string, FeeEstimateMode& fee_estimate_mode);
 std::string StringForFeeReason(FeeReason reason);
-std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason);
 std::string FeeModes(const std::string& delimiter);
 std::string FeeModeInfo(std::pair<std::string, FeeEstimateMode>& mode);
 std::string FeeModesDetail(std::string default_info);

src/policy/fees/block_policy_estimator.cpp

@@ -48,6 +48,23 @@ std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon)
     assert(false);
 }
 
+std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason)
+{
+    switch (reason) {
+    case BlockPolicyEstimateReason::NONE:
+        return "None";
+    case BlockPolicyEstimateReason::HALF_ESTIMATE:
+        return "Half Target 60% Threshold";
+    case BlockPolicyEstimateReason::FULL_ESTIMATE:
+        return "Target 85% Threshold";
+    case BlockPolicyEstimateReason::DOUBLE_ESTIMATE:
+        return "Double Target 95% Threshold";
+    case BlockPolicyEstimateReason::CONSERVATIVE:
+        return "Conservative Double Target longer horizon";
+    } // no default case, so the compiler can warn about missing cases
+    assert(false);
+}
+
 namespace {
 
 struct EncodedDoubleFormatter
@@ -872,11 +889,12 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
 {
     LOCK(m_cs_fee_estimator);
 
-    if (feeCalc) {
-        feeCalc->desiredTarget = confTarget;
-        feeCalc->returnedTarget = confTarget;
-        feeCalc->best_height = nBestSeenHeight;
-    }
+    FeeCalculation temp_fee_calc;
+    if (!feeCalc) feeCalc = &temp_fee_calc;
+
+    feeCalc->desiredTarget = confTarget;
+    feeCalc->returnedTarget = confTarget;
+    feeCalc->best_height = nBestSeenHeight;
 
     double median = -1;
     EstimationResult tempResult;
@@ -893,7 +911,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
     if ((unsigned int)confTarget > maxUsableEstimate) {
         confTarget = maxUsableEstimate;
     }
-    if (feeCalc) feeCalc->returnedTarget = confTarget;
+    feeCalc->returnedTarget = confTarget;
 
     if (confTarget <= 1) return CFeeRate(0); // error condition
 
@@ -917,41 +935,42 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
      * See: https://github.com/bitcoin/bitcoin/issues/11800#issuecomment-349697807
      */
     double halfEst = estimateCombinedFee(confTarget/2, HALF_SUCCESS_PCT, true, &tempResult);
-    if (feeCalc) {
-        feeCalc->est = tempResult;
-        feeCalc->reason = BlockPolicyEstimateReason::HALF_ESTIMATE;
-    }
+    feeCalc->est = tempResult;
+    feeCalc->reason = BlockPolicyEstimateReason::HALF_ESTIMATE;
     median = halfEst;
     double actualEst = estimateCombinedFee(confTarget, SUCCESS_PCT, true, &tempResult);
     if (actualEst > median) {
         median = actualEst;
-        if (feeCalc) {
-            feeCalc->est = tempResult;
-            feeCalc->reason = BlockPolicyEstimateReason::FULL_ESTIMATE;
-        }
+        feeCalc->est = tempResult;
+        feeCalc->reason = BlockPolicyEstimateReason::FULL_ESTIMATE;
     }
     double doubleEst = estimateCombinedFee(2 * confTarget, DOUBLE_SUCCESS_PCT, !conservative, &tempResult);
     if (doubleEst > median) {
         median = doubleEst;
-        if (feeCalc) {
-            feeCalc->est = tempResult;
-            feeCalc->reason = BlockPolicyEstimateReason::DOUBLE_ESTIMATE;
-        }
+        feeCalc->est = tempResult;
+        feeCalc->reason = BlockPolicyEstimateReason::DOUBLE_ESTIMATE;
     }
 
     if (conservative || median == -1) {
         double consEst =  estimateConservativeFee(2 * confTarget, &tempResult);
         if (consEst > median) {
             median = consEst;
-            if (feeCalc) {
-                feeCalc->est = tempResult;
-                feeCalc->reason = BlockPolicyEstimateReason::CONSERVATIVE;
-            }
+            feeCalc->est = tempResult;
+            feeCalc->reason = BlockPolicyEstimateReason::CONSERVATIVE;
         }
     }
 
     if (median < 0) return CFeeRate(0); // error condition
 
+    LogDebug(BCLog::ESTIMATEFEE, "estimateSmartFee Selected feerate: %g Tgt: %d (requested %d) Reason: \"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)",
+             median, feeCalc->returnedTarget, feeCalc->desiredTarget, StringForBlockPolicyEstimateReason(feeCalc->reason), feeCalc->est.decay,
+             feeCalc->est.pass.start, feeCalc->est.pass.end,
+             (feeCalc->est.pass.totalConfirmed + feeCalc->est.pass.inMempool + feeCalc->est.pass.leftMempool) > 0.0 ? 100 * feeCalc->est.pass.withinTarget / (feeCalc->est.pass.totalConfirmed + feeCalc->est.pass.inMempool + feeCalc->est.pass.leftMempool) : 0.0,
+             feeCalc->est.pass.withinTarget, feeCalc->est.pass.totalConfirmed, feeCalc->est.pass.inMempool, feeCalc->est.pass.leftMempool,
+             feeCalc->est.fail.start, feeCalc->est.fail.end,
+             (feeCalc->est.fail.totalConfirmed + feeCalc->est.fail.inMempool + feeCalc->est.fail.leftMempool) > 0.0 ? 100 * feeCalc->est.fail.withinTarget / (feeCalc->est.fail.totalConfirmed + feeCalc->est.fail.inMempool + feeCalc->est.fail.leftMempool) : 0.0,
+             feeCalc->est.fail.withinTarget, feeCalc->est.fail.totalConfirmed, feeCalc->est.fail.inMempool, feeCalc->est.fail.leftMempool);
+
     return CFeeRate(llround(median));
 }
 

src/policy/fees/block_policy_estimator.h

@@ -64,6 +64,8 @@ enum class BlockPolicyEstimateReason {
     CONSERVATIVE,
 };
 
+std::string StringForBlockPolicyEstimateReason(BlockPolicyEstimateReason reason);
+
 /* Used to return detailed information about a feerate bucket */
 struct EstimatorBucket
 {

src/test/fuzz/fees.cpp

@@ -15,7 +15,6 @@
 #include <vector>
 
 using common::StringForFeeReason;
-using common::StringForBlockPolicyEstimateReason;
 
 FUZZ_TARGET(fees)
 {