Skip to content

Commit

Permalink
Merge bitcoin#12336: Remove deprecated rpc options
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@laanwj @PastaPastaPasta
laanwj authored and PastaPastaPasta committed Jun 10, 2020
1 parent 3773cad commit 8254d30
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 167 deletions.
1 change: 0 additions & 1 deletion src/rpc/client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down
47 changes: 4 additions & 43 deletions src/rpc/mining.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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", "")
Expand All @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"} },
Expand Down
12 changes: 1 addition & 11 deletions src/rpc/misc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()));
}
}

Expand Down
9 changes: 0 additions & 9 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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\\\"]\"") +
Expand Down Expand Up @@ -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()));
Expand Down
22 changes: 11 additions & 11 deletions test/functional/deprecated_rpc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Loading

0 comments on commit 8254d30

Please sign in to comment.