Merge #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. (#11031)
  - the `errors` field returned by `getmininginfo` has been deprecated and replaced by a `warning` field. (#10858)
  - providing addresses as inputs to `createmultisig` has been deprecated. Users should use `addmultisigaddress` instead (#11415)
  - The return format from `addmultisigaddress` has changed (#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 #11739 and #11398 have been merged and the segwit test code tidied up)

Tree-SHA512: 8ffaa5f6094131339b9e9e468e8b141de4b144697d2271efa2992b80b12eb97849ade3da8df5c1c9400ed4c04e6a029926550a3e5846d2029b644f9e84ac7124
This commit is contained in:
Wladimir J. van der Laan 2018-02-08 15:26:20 +01:00
commit 3843780fd8
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D
6 changed files with 87 additions and 170 deletions

View File

@ -114,7 +114,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" },

View File

@ -201,7 +201,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 bitcoind is started with -deprecatedrpc=getmininginfo\n"
"}\n"
"\nExamples:\n"
+ HelpExampleCli("getmininginfo", "")
@ -219,11 +218,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")));
}
return obj;
}
@ -772,43 +767,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. Uses virtual transaction size of transaction\n"
"as defined in BIP 141 (witness data is discounted).\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 bitcoind 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)
@ -986,7 +946,7 @@ static const CRPCCommand commands[] =
{ "generating", "generatetoaddress", &generatetoaddress, {"nblocks","address","maxtries"} },
{ "util", "estimatefee", &estimatefee, {"nblocks"} },
{ "hidden", "estimatefee", &estimatefee, {} },
{ "util", "estimatesmartfee", &estimatesmartfee, {"conf_target", "estimate_mode"} },
{ "hidden", "estimaterawfee", &estimaterawfee, {"conf_target", "threshold"} },

View File

@ -262,9 +262,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 bitcoind 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"
@ -297,15 +294,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 bitcoind 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()));
}
}

View File

@ -1192,10 +1192,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 bitcoind with -deprecatedrpc=addmultisigaddress).\n"
" clients should transition to the new output api before upgrading to v0.17.\n"
"\"address\" (string) A bitcoin address associated with the keys.\n"
"\nExamples:\n"
"\nAdd a multisig address from 2 addresses\n"
+ HelpExampleCli("addmultisigaddress", "2 \"[\\\"16sSauSf5pF2UkUwvKGq4qjNRzBZYqgEL5\\\",\\\"171sgjn4YtPu27adkKGrdDwzRTxnRkBfKV\\\"]\"") +
@ -1238,11 +1234,6 @@ UniValue addmultisigaddress(const JSONRPCRequest& request)
CTxDestination dest = pwallet->AddAndGetDestinationForScript(inner, output_type);
pwallet->SetAddressBook(dest, strAccount, "send");
// Return old style interface
if (IsDeprecatedRPCEnabled("addmultisigaddress")) {
return EncodeDestination(dest);
}
UniValue result(UniValue::VOBJ);
result.pushKV("address", EncodeDestination(dest));
result.pushKV("redeemScript", HexStr(inner.begin(), inner.end()));
@ -3260,8 +3251,8 @@ UniValue bumpfee(const JSONRPCRequest& request)
"If the change output is not big enough to cover the increased fee, the command will currently fail\n"
"instead of adding new inputs to compensate. (A future implementation could improve this.)\n"
"The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n"
"By default, the new fee will be calculated automatically using estimatefee.\n"
"The user can specify a confirmation target for estimatefee.\n"
"By default, the new fee will be calculated automatically using estimatesmartfee.\n"
"The user can specify a confirmation target for estimatesmartfee.\n"
"Alternatively, the user can specify totalFee, or use RPC settxfee to set a higher fee rate.\n"
"At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n"
"returned by getnetworkinfo) to enter the node's mempool.\n"

View File

@ -3,35 +3,43 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test fee estimation code."""
from decimal import Decimal
import random
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *
from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
from test_framework.mininode import CTransaction, CTxIn, CTxOut, COutPoint, ToHex, COIN
from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than,
assert_greater_than_or_equal,
connect_nodes,
satoshi_round,
sync_blocks,
sync_mempools,
)
# Construct 2 trivial P2SH's and the ScriptSigs that spend them
# So we can create many transactions without needing to spend
# time signing.
redeem_script_1 = CScript([OP_1, OP_DROP])
redeem_script_2 = CScript([OP_2, OP_DROP])
P2SH_1 = CScript([OP_HASH160, hash160(redeem_script_1), OP_EQUAL])
P2SH_2 = CScript([OP_HASH160, hash160(redeem_script_2), OP_EQUAL])
REDEEM_SCRIPT_1 = CScript([OP_1, OP_DROP])
REDEEM_SCRIPT_2 = CScript([OP_2, OP_DROP])
P2SH_1 = CScript([OP_HASH160, hash160(REDEEM_SCRIPT_1), OP_EQUAL])
P2SH_2 = CScript([OP_HASH160, hash160(REDEEM_SCRIPT_2), OP_EQUAL])
# Associated ScriptSig's to spend satisfy P2SH_1 and P2SH_2
SCRIPT_SIG = [CScript([OP_TRUE, redeem_script_1]), CScript([OP_TRUE, redeem_script_2])]
global log
SCRIPT_SIG = [CScript([OP_TRUE, REDEEM_SCRIPT_1]), CScript([OP_TRUE, REDEEM_SCRIPT_2])]
def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee_increment):
"""
Create and send a transaction with a random fee.
"""Create and send a transaction with a random fee.
The transaction pays to a trivial P2SH script, and assumes that its inputs
are of the same form.
The function takes a list of confirmed outputs and unconfirmed outputs
and attempts to use the confirmed list first for its inputs.
It adds the newly created outputs to the unconfirmed list.
Returns (raw transaction, fee)
"""
Returns (raw transaction, fee)."""
# It's best to exponentially distribute our random fees
# because the buckets are exponentially spaced.
# Exponentially distributed from 1-128 * fee_increment
@ -64,13 +72,13 @@ def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee
return (ToHex(tx), fee)
def split_inputs(from_node, txins, txouts, initial_split=False):
"""
We need to generate a lot of inputs so we can generate a ton of transactions.
"""Generate a lot of inputs so we can generate a ton of transactions.
This function takes an input from txins, and creates and sends a transaction
which splits the value into 2 outputs which are appended to txouts.
Previously this was designed to be small inputs so they wouldn't have
a high coin age when the notion of priority still existed.
"""
a high coin age when the notion of priority still existed."""
prevtxout = txins.pop()
tx = CTransaction()
tx.vin.append(CTxIn(COutPoint(int(prevtxout["txid"], 16), prevtxout["vout"]), b""))
@ -91,54 +99,28 @@ def split_inputs(from_node, txins, txouts, initial_split = False):
txouts.append({"txid": txid, "vout": 0, "amount": half_change})
txouts.append({"txid": txid, "vout": 1, "amount": rem_change})
def check_estimates(node, fees_seen, max_invalid, print_estimates = True):
"""
This function calls estimatefee and verifies that the estimates
meet certain invariants.
"""
all_estimates = [ node.estimatefee(i) for i in range(1,26) ]
if print_estimates:
log.info([str(all_estimates[e-1]) for e in [1,2,3,6,15,25]])
def check_estimates(node, fees_seen, max_invalid):
"""Call estimatesmartfee and verify that the estimates meet certain invariants."""
delta = 1.0e-6 # account for rounding error
last_e = max(fees_seen)
for e in [x for x in all_estimates if x >= 0]:
# Estimates should be within the bounds of what transactions fees actually were:
if float(e)+delta < min(fees_seen) or float(e)-delta > max(fees_seen):
last_feerate = float(max(fees_seen))
all_smart_estimates = [node.estimatesmartfee(i) for i in range(1, 26)]
for i, e in enumerate(all_smart_estimates): # estimate is for i+1
feerate = float(e["feerate"])
assert_greater_than(feerate, 0)
if feerate + delta < min(fees_seen) or feerate - delta > max(fees_seen):
raise AssertionError("Estimated fee (%f) out of range (%f,%f)"
%(float(e), min(fees_seen), max(fees_seen)))
# Estimates should be monotonically decreasing
if float(e)-delta > last_e:
% (feerate, min(fees_seen), max(fees_seen)))
if feerate - delta > last_feerate:
raise AssertionError("Estimated fee (%f) larger than last fee (%f) for lower number of confirms"
%(float(e),float(last_e)))
last_e = e
valid_estimate = False
invalid_estimates = 0
for i,e in enumerate(all_estimates): # estimate is for i+1
if e >= 0:
valid_estimate = True
if i >= 13: # for n>=14 estimatesmartfee(n/2) should be at least as high as estimatefee(n)
assert(node.estimatesmartfee((i+1)//2)["feerate"] > float(e) - delta)
% (feerate, last_feerate))
last_feerate = feerate
if i == 0:
assert_equal(e["blocks"], 2)
else:
invalid_estimates += 1
# estimatesmartfee should still be valid
approx_estimate = node.estimatesmartfee(i+1)["feerate"]
answer_found = node.estimatesmartfee(i+1)["blocks"]
assert(approx_estimate > 0)
assert(answer_found > i+1)
# Once we're at a high enough confirmation count that we can give an estimate
# We should have estimates for all higher confirmation counts
if valid_estimate:
raise AssertionError("Invalid estimate appears at higher confirm count than valid estimate")
# Check on the expected number of different confirmation counts
# that we might not have valid estimates for
if invalid_estimates > max_invalid:
raise AssertionError("More than (%d) invalid estimates"%(max_invalid))
return all_estimates
assert_greater_than_or_equal(i + 1, e["blocks"])
class EstimateFeeTest(BitcoinTestFramework):
def set_test_params(self):
@ -151,7 +133,7 @@ class EstimateFeeTest(BitcoinTestFramework):
which we will use to generate our transactions.
"""
self.add_nodes(3, extra_args=[["-maxorphantx=1000", "-whitelist=127.0.0.1"],
["-blockmaxsize=17000", "-maxorphantx=1000", "-deprecatedrpc=estimatefee"],
["-blockmaxsize=17000", "-maxorphantx=1000"],
["-blockmaxsize=8000", "-maxorphantx=1000"]])
# Use node0 to mine blocks for input splitting
# Node1 mines small blocks but that are bigger than the expected transaction rate.
@ -160,7 +142,6 @@ class EstimateFeeTest(BitcoinTestFramework):
# Node2 is a stingy miner, that
# produces too small blocks (room for only 55 or so transactions)
def transact_and_mine(self, numblocks, mining_node):
min_fee = Decimal("0.00001")
# We will now mine numblocks blocks generating on average 100 transactions between each block
@ -191,10 +172,6 @@ class EstimateFeeTest(BitcoinTestFramework):
self.log.info("This test is time consuming, please be patient")
self.log.info("Splitting inputs so we can generate tx's")
# Make log handler available to helper functions
global log
log = self.log
# Start node0
self.start_node(0)
self.txouts = []

View File

@ -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()