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

4e28753f60 feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c1 init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202 Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957 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 4e28753f60

Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8

src/node/context.cpp

@@ -8,6 +8,7 @@
 #include <interfaces/chain.h>
 #include <net.h>
 #include <net_processing.h>
+#include <policy/fees.h>
 #include <scheduler.h>
 #include <txmempool.h>
 

src/node/context.h

@@ -12,6 +12,7 @@
 
 class ArgsManager;
 class BanMan;
+class CBlockPolicyEstimator;
 class CConnman;
 class CScheduler;
 class CTxMemPool;
@@ -36,6 +37,7 @@ class WalletClient;
 struct NodeContext {
     std::unique_ptr<CConnman> connman;
     std::unique_ptr<CTxMemPool> mempool;
+    std::unique_ptr<CBlockPolicyEstimator> fee_estimator;
     std::unique_ptr<PeerManager> peerman;
     ChainstateManager* chainman{nullptr}; // Currently a raw pointer because the memory is not managed by this struct
     std::unique_ptr<BanMan> banman;

src/node/interfaces.cpp

@@ -221,15 +221,6 @@ public:
         }
     }
     bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); }
-    CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override
-    {
-        FeeCalculation fee_calc;
-        CFeeRate result = ::feeEstimator.estimateSmartFee(num_blocks, &fee_calc, conservative);
-        if (returned_target) {
-            *returned_target = fee_calc.returnedTarget;
-        }
-        return result;
-    }
     CFeeRate getDustRelayFee() override { return ::dustRelayFee; }
     UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override
     {
@@ -601,11 +592,13 @@ public:
     }
     CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override
     {
-        return ::feeEstimator.estimateSmartFee(num_blocks, calc, conservative);
+        if (!m_node.fee_estimator) return {};
+        return m_node.fee_estimator->estimateSmartFee(num_blocks, calc, conservative);
     }
     unsigned int estimateMaxBlocks() override
     {
-        return ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
+        if (!m_node.fee_estimator) return 0;
+        return m_node.fee_estimator->HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
     }
     CFeeRate mempoolMinFee() override
     {