Merge 23387 via rpc_savefeeestimates

src/policy/fees.cpp

@@ -947,13 +947,15 @@ void CBlockPolicyEstimator::Flush() {
     FlushFeeEstimates();
 }
 
-void CBlockPolicyEstimator::FlushFeeEstimates()
+bool CBlockPolicyEstimator::FlushFeeEstimates() const
 {
     AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "wb")};
     if (est_file.IsNull() || !Write(est_file) || est_file.fclose() != 0) {
         LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
+        return false;
     } else {
         LogPrintf("Flushed fee estimates to %s.\n", fs::PathToString(m_estimation_filepath.filename()));
+        return true;
     }
 }
 

src/policy/fees.h

@@ -256,7 +256,7 @@ public:
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Record current fee estimations. */
-    void FlushFeeEstimates()
+    bool FlushFeeEstimates() const
         EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
 
     /** Calculates the age of the file, since last modified */

src/rpc/fees.cpp

@@ -214,10 +214,36 @@ static RPCHelpMan estimaterawfee()
     };
 }
 
+static RPCHelpMan savefeeestimates()
+{
+    return RPCHelpMan{"savefeeestimates",
+                "\nDumps the fee estimates to disk. It will fail until the previous dump is fully loaded.\n",
+                {},
+                RPCResult{RPCResult::Type::NONE, "", ""},
+                RPCExamples{
+                    HelpExampleCli("savefeeestimates", "")
+            + HelpExampleRpc("savefeeestimates", "")
+                },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    static Mutex dump_mutex;
+    LOCK(dump_mutex);
+    CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
+
+    if (!fee_estimator.FlushFeeEstimates()) {
+        throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump fee estimates to disk");
+    }
+
+    return NullUniValue;
+},
+    };
+}
+
 void RegisterFeeRPCCommands(CRPCTable& t)
 {
     static const CRPCCommand commands[]{
         {"util", &estimatesmartfee},
+        {"util", &savefeeestimates},
         {"hidden", &estimaterawfee},
     };
     for (const auto& c : commands) {

src/test/fuzz/rpc.cpp

@@ -85,6 +85,7 @@ const std::vector<std::string> RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{
     "importwallet", // avoid reading from disk
     "loadtxoutset",   // avoid reading from disk
     "loadwallet",   // avoid reading from disk
+    "savefeeestimates",      // disabled as a precautionary measure: may take a file path argument in the future
     "savemempool",           // disabled as a precautionary measure: may take a file path argument in the future
     "setban",                // avoid DNS lookups
     "stop",                  // avoid shutdown state

test/functional/feature_fee_estimates_persist.py

@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+# Copyright (c) 2014-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test fee estimates persistence.
+
+By default, bitcoind will dump fee estimates on shutdown and
+then reload it on startup.
+
+Test is as follows:
+
+  - start node0
+  - call the savefeeestimates RPC and verify the RPC succeeds and
+    that the file exists
+  - make the file read only and attempt to call the savefeeestimates RPC
+    with the expecation that it will fail
+  - move the read only file and shut down the node, verify the node writes
+    on shutdown a file that is identical to the one we saved via the RPC
+
+"""
+
+import filecmp
+import os
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_raises_rpc_error
+
+
+class FeeEstimatesPersistTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def run_test(self):
+        fee_estimatesdat = os.path.join(self.nodes[0].chain_path, 'fee_estimates.dat')
+        self.log.debug('Verify the fee_estimates.dat file does not exists on start up')
+        assert not os.path.isfile(fee_estimatesdat)
+        self.nodes[0].savefeeestimates()
+        self.log.debug('Verify the fee_estimates.dat file exists after calling savefeeestimates RPC')
+        assert os.path.isfile(fee_estimatesdat)
+        self.log.debug("Prevent bitcoind from writing fee_estimates.dat to disk. Verify that `savefeeestimates` fails")
+        fee_estimatesdatold = fee_estimatesdat + '.old'
+        os.rename(fee_estimatesdat, fee_estimatesdatold)
+        os.mkdir(fee_estimatesdat)
+        assert_raises_rpc_error(-1, "Unable to dump fee estimates to disk", self.nodes[0].savefeeestimates)
+        os.rmdir(fee_estimatesdat)
+        self.stop_nodes()
+        self.log.debug("Verify that fee_estimates are written on shutdown")
+        assert os.path.isfile(fee_estimatesdat)
+        self.log.debug("Verify that the fee estimates from a shutdown are identical from the ones from savefeeestimates")
+        assert filecmp.cmp(fee_estimatesdat, fee_estimatesdatold)
+
+
+if __name__ == "__main__":
+    FeeEstimatesPersistTest(__file__).main()

test/functional/test_runner.py

@@ -176,6 +176,7 @@ BASE_SCRIPTS = [
     'wallet_fast_rescan.py --descriptors',
     'wallet_gethdkeys.py --descriptors',
     'wallet_createwalletdescriptor.py --descriptors',
+    'feature_fee_estimates_persist.py',
     'interface_zmq.py',
     'rpc_invalid_address_message.py',
     'rpc_validateaddress.py',