Merge pull request #6423 from str4d/6420-allowdeprecated-in-config-file

Load `-allowdeprecated` settings after reading the config file

qa/rpc-tests/wallet_deprecation.py

@@ -12,22 +12,48 @@ from test_framework.util import (
 )
 from test_framework.authproxy import JSONRPCException
 
+import os.path
+
+# Pick a subset of the deprecated RPC methods to test with. This test assumes that
+# the deprecation feature name is the same as the RPC method name, and that the RPC
+# method works without any arguments.
+DEFAULT_ENABLED = [
+    "z_gettotalbalance",
+]
+DEFAULT_DISABLED = [
+    "getnewaddress",
+    "z_getnewaddress",
+]
+
 # Test wallet address behaviour across network upgrades
 class WalletDeprecationTest(BitcoinTestFramework):
     def __init__(self):
         super().__init__()
         self.num_nodes = 1
 
-    def setup_network(self):
+    def setup_chain(self):
-        self.setup_network_internal([])
+        super().setup_chain()
+        # Save a copy of node 0's zcash.conf
+        with open(os.path.join(self.options.tmpdir, "node0", "zcash.conf"), 'r', encoding='utf8') as f:
+            self.conf_lines = f.readlines()
 
-    def setup_network_internal(self, allowed_deprecated = []):
+    def setup_network(self):
+        self.setup_network_with_args([])
+
+    def setup_network_with_args(self, allowed_deprecated):
         dep_args = ["-allowdeprecated=" + v for v in allowed_deprecated]
 
         self.nodes = start_nodes(
             self.num_nodes, self.options.tmpdir,
             extra_args=[dep_args] * self.num_nodes)
 
+    def setup_network_with_config(self, allowed_deprecated):
+        conf_lines = self.conf_lines + ["allowdeprecated={}\n".format(v) for v in allowed_deprecated]
+        with open(os.path.join(self.options.tmpdir, "node0", "zcash.conf"), 'w', encoding='utf8') as f:
+            f.writelines(conf_lines)
+
+        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir)
+
     def verify_enabled(self, function):
         try:
             getattr(self.nodes[0], function)()
@@ -50,16 +76,17 @@ class WalletDeprecationTest(BitcoinTestFramework):
                 "failed with '%s'" % errorString if len(errorString) > 0 else "succeeded",
             ))
 
-    def run_test(self):
+    def test_case(self, start_mode, features_to_allow, expected_state):
-        # Pick a subset of the deprecated RPC methods to test with. This test assumes that
+        stop_nodes(self.nodes)
-        # the deprecation feature name is the same as the RPC method name.
+        wait_bitcoinds()
-        DEFAULT_ENABLED = [
+        start_mode(features_to_allow)
-        ]
-        DEFAULT_DISABLED = [
-            "getnewaddress",
-            "z_getnewaddress",
-        ]
 
+        for function in DEFAULT_ENABLED:
+            expected_state(function)
+        for function in DEFAULT_DISABLED:
+            expected_state(function)
+
+    def run_test(self):
         # RPC methods that are deprecated but enabled by default should succeed
         for function in DEFAULT_ENABLED:
             self.verify_enabled(function)
@@ -68,25 +95,12 @@ class WalletDeprecationTest(BitcoinTestFramework):
         for function in DEFAULT_DISABLED:
             self.verify_disabled(function)
 
-        # restart with a specific selection of deprecated methods enabled
+        for start_mode in (self.setup_network_with_args, self.setup_network_with_config):
-        stop_nodes(self.nodes)
+            # restart with a specific selection of deprecated methods enabled
-        wait_bitcoinds()
+            self.test_case(start_mode, DEFAULT_DISABLED, self.verify_enabled)
-        self.setup_network_internal(DEFAULT_DISABLED)
 
-        for function in DEFAULT_ENABLED:
+            # restart with no deprecated methods enabled
-            self.verify_enabled(function)
+            self.test_case(start_mode, ["none"], self.verify_disabled)
-        for function in DEFAULT_DISABLED:
-            self.verify_enabled(function)
-
-        # restart with no deprecated methods enabled
-        stop_nodes(self.nodes)
-        wait_bitcoinds()
-        self.setup_network_internal(["none"])
-
-        for function in DEFAULT_ENABLED:
-            self.verify_disabled(function)
-        for function in DEFAULT_DISABLED:
-            self.verify_disabled(function)
 
 if __name__ == '__main__':
     WalletDeprecationTest().main()

src/bitcoind.cpp

@@ -94,14 +94,6 @@ bool AppInit(int argc, char* argv[])
         return true;
     }
 
-    // Handle setting of allowed-deprecated features as early as possible
-    // so that it's possible for other initialization steps to respect them.
-    auto deprecationError = SetAllowedDeprecatedFeaturesFromCLIArgs();
-    if (deprecationError.has_value()) {
-        fprintf(stderr, "%s", deprecationError.value().c_str());
-        return false;
-    }
-
     try
     {
         if (!fs::is_directory(GetDataDir(false)))
@@ -143,6 +135,14 @@ bool AppInit(int argc, char* argv[])
             return false;
         }
 
+        // Handle setting of allowed-deprecated features as early as possible
+        // so that it's possible for other initialization steps to respect them.
+        auto deprecationError = LoadAllowedDeprecatedFeatures();
+        if (deprecationError.has_value()) {
+            fprintf(stderr, "%s", deprecationError.value().c_str());
+            return false;
+        }
+
         // Command-line RPC
         bool fCommandLine = false;
         for (int i = 1; i < argc; i++)

src/deprecation.cpp

@@ -61,7 +61,7 @@ void EnforceNodeDeprecation(int nHeight, bool forceLogging, bool fThread) {
     }
 }
 
-std::optional<std::string> SetAllowedDeprecatedFeaturesFromCLIArgs() {
+std::optional<std::string> LoadAllowedDeprecatedFeatures() {
     auto args = GetMultiArg("-allowdeprecated");
     std::set<std::string> allowdeprecated(args.begin(), args.end());
 

src/deprecation.h

@@ -71,13 +71,13 @@ extern bool fEnableWalletTxVJoinSplit;
 void EnforceNodeDeprecation(int nHeight, bool forceLogging=false, bool fThread=true);
 
 /**
- * Checks command-line arguments for enabling and/or disabling of deprecated
+ * Checks config options for enabling and/or disabling of deprecated
  * features and sets flags that enable deprecated features accordingly.
  *
  * @return std::nullopt if successful, or an error message indicating what
  * values are permitted for `-allowdeprecated`.
  */
-std::optional<std::string> SetAllowedDeprecatedFeaturesFromCLIArgs();
+std::optional<std::string> LoadAllowedDeprecatedFeatures();
 
 /**
  * Returns a comma-separated list of the valid arguments to the -allowdeprecated