Merge bitcoin#12336: Remove deprecated rpc options

db1cbcc [RPC] Remove deprecated addmultisigaddress return format (John Newbery)
cb28a0b [RPC] Remove deprecated createmultisig object (John Newbery)
ed45c82 [tests] Remove test for deprecated createmultsig option (John Newbery)
d066a1c [rpc] Remove deprecated getmininginfo RPC option (John Newbery)
c6f09c2 [rpc] remove deprecated estimatefee RPC (John Newbery)
a8e437a [tests] Remove estimatefee from rpc_deprecated.py test (John Newbery)
a5623b1 [tests] Remove tests for deprecated estimatefee RPC (John Newbery)
d119f2e [tests] Fix style warnings in feature_fee_estimation.py (John Newbery)

Pull request description:

  There were some RPC/RPC options deprecated in v0.16. Those can now be removed from master since v0.16 has been branched.

  - `estimatefee` RPC has been removed. The `feature_fee_estimation.py` test has been updated to remove the RPC, but doesn't yet have good coverage of the replacement RPC `estimatesmartfee`. Improving the test coverage should be done in a new PR. (bitcoin#11031)
  - the `errors` field returned by `getmininginfo` has been deprecated and replaced by a `warning` field. (bitcoin#10858)
  - providing addresses as inputs to `createmultisig` has been deprecated. Users should use `addmultisigaddress` instead (bitcoin#11415)
  - The return format from `addmultisigaddress` has changed (bitcoin#11415)

  `getwitnessaddress` was also deprecated in v0.16 and can be removed, but many tests are using that RPC, so it's a larger job to remove. It should be removed in a separate PR (possibly after bitcoin#11739 and bitcoin#11398 have been merged and the segwit test code tidied up)

Tree-SHA512: 8ffaa5f6094131339b9e9e468e8b141de4b144697d2271efa2992b80b12eb97849ade3da8df5c1c9400ed4c04e6a029926550a3e5846d2029b644f9e84ac7124

src/rpc/client.cpp

@@ -137,7 +137,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "pruneblockchain", 0, "height" },
     { "keypoolrefill", 0, "newsize" },
     { "getrawmempool", 0, "verbose" },
-    { "estimatefee", 0, "nblocks" },
     { "estimatesmartfee", 0, "conf_target" },
     { "estimaterawfee", 0, "conf_target" },
     { "estimaterawfee", 1, "threshold" },

src/rpc/mining.cpp

@@ -213,7 +213,6 @@ UniValue getmininginfo(const JSONRPCRequest& request)
             "  \"pooledtx\": n              (numeric) The size of the mempool\n"
             "  \"chain\": \"xxxx\",           (string) current network name as defined in BIP70 (main, test, regtest)\n"
             "  \"warnings\": \"...\"          (string) any network and blockchain warnings\n"
-            "  \"errors\": \"...\"            (string) DEPRECATED. Same as warnings. Only shown when dashd is started with -deprecatedrpc=getmininginfo\n"
             "}\n"
             "\nExamples:\n"
             + HelpExampleCli("getmininginfo", "")
@@ -231,11 +230,7 @@ UniValue getmininginfo(const JSONRPCRequest& request)
     obj.push_back(Pair("networkhashps",    getnetworkhashps(request)));
     obj.push_back(Pair("pooledtx",         (uint64_t)mempool.size()));
     obj.push_back(Pair("chain",            Params().NetworkIDString()));
-    if (IsDeprecatedRPCEnabled("getmininginfo")) {
-        obj.push_back(Pair("errors",       GetWarnings("statusbar")));
-    } else {
-        obj.push_back(Pair("warnings",     GetWarnings("statusbar")));
-    }
+    obj.push_back(Pair("warnings",         GetWarnings("statusbar")));
     return obj;
 }
 
@@ -799,42 +794,8 @@ UniValue submitblock(const JSONRPCRequest& request)
 
 UniValue estimatefee(const JSONRPCRequest& request)
 {
-    if (request.fHelp || request.params.size() != 1)
-        throw std::runtime_error(
-            "estimatefee nblocks\n"
-            "\nDEPRECATED. Please use estimatesmartfee for more intelligent estimates."
-            "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n"
-            "confirmation within nblocks blocks.\n"
-            "\nArguments:\n"
-            "1. nblocks     (numeric, required)\n"
-            "\nResult:\n"
-            "n              (numeric) estimated fee-per-kilobyte\n"
-            "\n"
-            "A negative value is returned if not enough transactions and blocks\n"
-            "have been observed to make an estimate.\n"
-            "-1 is always returned for nblocks == 1 as it is impossible to calculate\n"
-            "a fee that is high enough to get reliably included in the next block.\n"
-            "\nExample:\n"
-            + HelpExampleCli("estimatefee", "6")
-            );
-
-    if (!IsDeprecatedRPCEnabled("estimatefee")) {
-        throw JSONRPCError(RPC_METHOD_DEPRECATED, "estimatefee is deprecated and will be fully removed in v0.17. "
-            "To use estimatefee in v0.16, restart dashd with -deprecatedrpc=estimatefee.\n"
-            "Projects should transition to using estimatesmartfee before upgrading to v0.17");
-    }
-
-    RPCTypeCheck(request.params, {UniValue::VNUM});
-
-    int nBlocks = request.params[0].get_int();
-    if (nBlocks < 1)
-        nBlocks = 1;
-
-    CFeeRate feeRate = ::feeEstimator.estimateFee(nBlocks);
-    if (feeRate == CFeeRate(0))
-        return -1.0;
-
-    return ValueFromAmount(feeRate.GetFeePerK());
+    throw JSONRPCError(RPC_METHOD_DEPRECATED, "estimatefee was removed in v0.17.\n"
+        "Clients should use estimatesmartfee.");
 }
 
 UniValue estimatesmartfee(const JSONRPCRequest& request)
@@ -1011,7 +972,7 @@ static const CRPCCommand commands[] =
     { "generating",         "generatetoaddress",      &generatetoaddress,      {"nblocks","address","maxtries"} },
 #endif // ENABLE_MINER
 
-    { "util",               "estimatefee",            &estimatefee,            {"nblocks"} },
+    { "hidden",             "estimatefee",            &estimatefee,            {} },
     { "util",               "estimatesmartfee",       &estimatesmartfee,       {"conf_target", "estimate_mode"} },
 
     { "hidden",             "estimaterawfee",         &estimaterawfee,         {"conf_target", "threshold"} },

src/rpc/misc.cpp

@@ -343,9 +343,6 @@ UniValue createmultisig(const JSONRPCRequest& request)
         std::string msg = "createmultisig nrequired [\"key\",...]\n"
             "\nCreates a multi-signature address with n signature of m keys required.\n"
             "It returns a json object with the address and redeemScript.\n"
-            "DEPRECATION WARNING: Using addresses with createmultisig is deprecated. Clients must\n"
-            "transition to using addmultisigaddress to create multisig addresses with addresses known\n"
-            "to the wallet before upgrading to v0.17. To use the deprecated functionality, start dashd with -deprecatedrpc=createmultisig\n"
             "\nArguments:\n"
             "1. nrequired                    (numeric, required) The number of required signatures out of the n keys or addresses.\n"
             "2. \"keys\"                       (string, required) A json array of hex-encoded public keys\n"
@@ -378,15 +375,8 @@ UniValue createmultisig(const JSONRPCRequest& request)
         if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) {
             pubkeys.push_back(HexToPubKey(keys[i].get_str()));
         } else {
-#ifdef ENABLE_WALLET
-            CWallet* const pwallet = GetWalletForJSONRPCRequest(request);
-            if (IsDeprecatedRPCEnabled("createmultisig") && EnsureWalletIsAvailable(pwallet, false)) {
-                pubkeys.push_back(AddrToPubKey(pwallet, keys[i].get_str()));
-            } else
-#endif
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\nNote that from v0.16, createmultisig no longer accepts addresses."
-            " Clients must transition to using addmultisigaddress to create multisig addresses with addresses known to the wallet before upgrading to v0.17."
-            " To use the deprecated functionality, start dashd with -deprecatedrpc=createmultisig", keys[i].get_str()));
+            " Users must use addmultisigaddress to create multisig addresses with addresses known to the wallet.", keys[i].get_str()));
         }
     }
 

src/wallet/rpcwallet.cpp

@@ -1235,10 +1235,6 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
             "  \"address\":\"multisigaddress\",    (string) The value of the new multisig address.\n"
             "  \"redeemScript\":\"script\"         (string) The string value of the hex-encoded redemption script.\n"
             "}\n"
-            "\nResult (DEPRECATED. To see this result in v0.16 instead, please start dashd with -deprecatedrpc=addmultisigaddress).\n"
-            "        clients should transition to the new output api before upgrading to v0.17.\n"
-            "\"address\"                         (string) A dash address associated with the keys.\n"
-
             "\nExamples:\n"
             "\nAdd a multisig address from 2 addresses\n"
             + HelpExampleCli("addmultisigaddress", "2 \"[\\\"Xt4qk9uKvQYAonVGSZNXqxeDmtjaEWgfrS\\\",\\\"XoSoWQkpgLpppPoyyzbUFh1fq2RBvW6UK2\\\"]\"") +
@@ -1274,11 +1270,6 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
 
     pwallet->SetAddressBook(innerID, strAccount, "send");
 
-    // Return old style interface
-    if (IsDeprecatedRPCEnabled("addmultisigaddress")) {
-        return EncodeDestination(innerID);
-    }
-
     UniValue result(UniValue::VOBJ);
     result.pushKV("address", EncodeDestination(innerID));
     result.pushKV("redeemScript", HexStr(inner.begin(), inner.end()));

test/functional/deprecated_rpc.py

@@ -4,24 +4,24 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test deprecation of RPC calls."""
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_raises_rpc_error
 
 class DeprecatedRpcTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.setup_clean_chain = True
-        self.extra_args = [[], ["-deprecatedrpc=estimatefee", "-deprecatedrpc=createmultisig"]]
+        self.extra_args = [[], []]
 
     def run_test(self):
-        self.log.info("estimatefee: Shows deprecated message")
-        assert_raises_rpc_error(-32, 'estimatefee is deprecated', self.nodes[0].estimatefee, 1)
-
-        self.log.info("Using -deprecatedrpc=estimatefee bypasses the error")
-        self.nodes[1].estimatefee(1)
-
-        self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
-        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
-        self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+        # This test should be used to verify correct behaviour of deprecated
+        # RPC methods with and without the -deprecatedrpc flags. For example:
+        #
+        # self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
+        # assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
+        # self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+        #
+        # There are currently no deprecated RPC methods in master, so this
+        # test is currently empty.
+        pass
 
 if __name__ == '__main__':
     DeprecatedRpcTest().main()