Merge #18766: Disable fee estimation in blocksonly mode (by removing the fee estimates global)

4e28753f60613ecd35cdef87bef5f99c302c3fbd feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c16997bdc7e22a20eca16e234290b7ff init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202bfb9d9b50800b8ffe3fead3f77f5fa Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957ab7e3b6aece82b9561774648094f54 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)

Pull request description:

  If the `blocksonly` mode is turned on after running with transaction
  relay enabled for a while, the fee estimation will serve outdated data
  to both the internal wallet and to external applications that might be
  feerate-sensitive and make use of `estimatesmartfee` (for example a
  Lightning Network node).

  This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.

  This fixes #16840, and closes #16890 which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
  > If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).

ACKs for top commit:
  MarcoFalke:
    re-ACK 4e28753f60 👋
  jnewbery:
    utACK 4e28753f60613ecd35cdef87bef5f99c302c3fbd

Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8

3 files changed, 23 insertions, 5 deletions

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 392073d047..57327e6004 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -17,6 +17,7 @@
 #include <node/coinstats.h>
 #include <node/context.h>
 #include <node/utxo_snapshot.h>
+#include <policy/fees.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
 #include <policy/rbf.h>
@@ -81,6 +82,15 @@ ChainstateManager& EnsureChainman(const util::Ref& context)
     return *node.chainman;
 }
 
+CBlockPolicyEstimator& EnsureFeeEstimator(const util::Ref& context)
+{
+    NodeContext& node = EnsureNodeContext(context);
+    if (!node.fee_estimator) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Fee estimation disabled");
+    }
+    return *node.fee_estimator;
+}
+
 /* Calculate the difficulty for a given block index.
  */
 double GetDifficulty(const CBlockIndex* blockindex)
diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h
index 5b362bf211..91766aacc9 100644
--- a/src/rpc/blockchain.h
+++ b/src/rpc/blockchain.h
@@ -15,6 +15,7 @@ extern RecursiveMutex cs_main;
 
 class CBlock;
 class CBlockIndex;
+class CBlockPolicyEstimator;
 class CTxMemPool;
 class ChainstateManager;
 class UniValue;
@@ -54,5 +55,6 @@ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES],
 NodeContext& EnsureNodeContext(const util::Ref& context);
 CTxMemPool& EnsureMemPool(const util::Ref& context);
 ChainstateManager& EnsureChainman(const util::Ref& context);
+CBlockPolicyEstimator& EnsureFeeEstimator(const util::Ref& context);
 
 #endif
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 7d45ad9434..798973850f 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -1059,7 +1059,10 @@ static RPCHelpMan estimatesmartfee()
 {
     RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR});
     RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
-    unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
+
+    CBlockPolicyEstimator& fee_estimator = EnsureFeeEstimator(request.context);
+
+    unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
     unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
     bool conservative = true;
     if (!request.params[1].isNull()) {
@@ -1073,7 +1076,7 @@ static RPCHelpMan estimatesmartfee()
     UniValue result(UniValue::VOBJ);
     UniValue errors(UniValue::VARR);
     FeeCalculation feeCalc;
-    CFeeRate feeRate = ::feeEstimator.estimateSmartFee(conf_target, &feeCalc, conservative);
+    CFeeRate feeRate = fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative);
     if (feeRate != CFeeRate(0)) {
         result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK()));
     } else {
@@ -1144,7 +1147,10 @@ static RPCHelpMan estimaterawfee()
 {
     RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true);
     RPCTypeCheckArgument(request.params[0], UniValue::VNUM);
-    unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
+
+    CBlockPolicyEstimator& fee_estimator = EnsureFeeEstimator(request.context);
+
+    unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
     unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
     double threshold = 0.95;
     if (!request.params[1].isNull()) {
@@ -1161,9 +1167,9 @@ static RPCHelpMan estimaterawfee()
         EstimationResult buckets;
 
         // Only output results for horizons which track the target
-        if (conf_target > ::feeEstimator.HighestTargetTracked(horizon)) continue;
+        if (conf_target > fee_estimator.HighestTargetTracked(horizon)) continue;
 
-        feeRate = ::feeEstimator.estimateRawFee(conf_target, threshold, horizon, &buckets);
+        feeRate = fee_estimator.estimateRawFee(conf_target, threshold, horizon, &buckets);
         UniValue horizon_result(UniValue::VOBJ);
         UniValue errors(UniValue::VARR);
         UniValue passbucket(UniValue::VOBJ);