diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 06781a701f98e4..274e6793f30ef4 100644 --- a/src/rpc/client.cpp +++ b/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" }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3f9987e2cea395..b437845f7c6701 100644 --- a/src/rpc/mining.cpp +++ b/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"} }, diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index ac4a9c2c76bb36..da683e2e0302de 100644 --- a/src/rpc/misc.cpp +++ b/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())); } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index eed197678c98f3..c6182834ca6aae 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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())); diff --git a/test/functional/deprecated_rpc.py b/test/functional/deprecated_rpc.py index d6f25158ef3077..90183474bbd24d 100755 --- a/test/functional/deprecated_rpc.py +++ b/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() diff --git a/test/functional/smartfees.py b/test/functional/smartfees.py index 05353a1162f96a..9d1e5fe9a95425 100755 --- a/test/functional/smartfees.py +++ b/test/functional/smartfees.py @@ -3,39 +3,47 @@ # 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 - rand_fee = float(fee_increment)*(1.1892**random.randint(0,28)) + rand_fee = float(fee_increment) * (1.1892 ** random.randint(0, 28)) # Total fee ranges from min_fee to min_fee + 127*fee_increment fee = min_fee - fee_increment + satoshi_round(rand_fee) tx = CTransaction() @@ -50,95 +58,69 @@ def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee total_in += t["amount"] tx.vin.append(CTxIn(COutPoint(int(t["txid"], 16), t["vout"]), b"")) if total_in <= amount + fee: - raise RuntimeError("Insufficient funds: need %d, have %d"%(amount+fee, total_in)) - tx.vout.append(CTxOut(int((total_in - amount - fee)*COIN), P2SH_1)) - tx.vout.append(CTxOut(int(amount*COIN), P2SH_2)) + raise RuntimeError("Insufficient funds: need %d, have %d" % (amount + fee, total_in)) + tx.vout.append(CTxOut(int((total_in - amount - fee) * COIN), P2SH_1)) + tx.vout.append(CTxOut(int(amount * COIN), P2SH_2)) # These transactions don't need to be signed, but we still have to insert # the ScriptSig that will satisfy the ScriptPubKey. for inp in tx.vin: inp.scriptSig = SCRIPT_SIG[inp.prevout.n] txid = from_node.sendrawtransaction(ToHex(tx), True) - unconflist.append({ "txid" : txid, "vout" : 0 , "amount" : total_in - amount - fee}) - unconflist.append({ "txid" : txid, "vout" : 1 , "amount" : amount}) + unconflist.append({"txid": txid, "vout": 0, "amount": total_in - amount - fee}) + unconflist.append({"txid": txid, "vout": 1, "amount": amount}) 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. +def split_inputs(from_node, txins, txouts, initial_split=False): + """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"")) - half_change = satoshi_round(prevtxout["amount"]/2) - rem_change = prevtxout["amount"] - half_change - Decimal("0.00001000") - tx.vout.append(CTxOut(int(half_change*COIN), P2SH_1)) - tx.vout.append(CTxOut(int(rem_change*COIN), P2SH_2)) + half_change = satoshi_round(prevtxout["amount"] / 2) + rem_change = prevtxout["amount"] - half_change - Decimal("0.00001000") + tx.vout.append(CTxOut(int(half_change * COIN), P2SH_1)) + tx.vout.append(CTxOut(int(rem_change * COIN), P2SH_2)) # If this is the initial split we actually need to sign the transaction # Otherwise we just need to insert the proper ScriptSig - if (initial_split) : + if (initial_split): completetx = from_node.signrawtransaction(ToHex(tx))["hex"] - else : + else: tx.vin[0].scriptSig = SCRIPT_SIG[prevtxout["vout"]] completetx = ToHex(tx) txid = from_node.sendrawtransaction(completetx, True) - txouts.append({ "txid" : txid, "vout" : 0 , "amount" : half_change}) - txouts.append({ "txid" : txid, "vout" : 1 , "amount" : rem_change}) + 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]]) - 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): +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_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 @@ def setup_network(self): which we will use to generate our transactions. """ self.add_nodes(3, extra_args=[["-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1", "-deprecatedrpc=estimatefee"], + ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], ["-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"]]) # 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 @@ def setup_network(self): # 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.0001") # We will now mine numblocks blocks generating on average 100 transactions between each block @@ -169,14 +150,14 @@ def transact_and_mine(self, numblocks, mining_node): # resorting to tx's that depend on the mempool when those run out for i in range(numblocks): random.shuffle(self.confutxo) - for j in range(random.randrange(100-50,100+50)): - from_index = random.randint(1,2) + for j in range(random.randrange(100 - 50, 100 + 50)): + from_index = random.randint(1, 2) (txhex, fee) = small_txpuzzle_randfee(self.nodes[from_index], self.confutxo, self.memutxo, Decimal("0.005"), min_fee, min_fee) tx_kbytes = (len(txhex) // 2) / 1000.0 - self.fees_per_kb.append(float(fee)/tx_kbytes) + self.fees_per_kb.append(float(fee) / tx_kbytes) self.sync_mempools(self.nodes[0:3], wait=.1) - mined = mining_node.getblock(mining_node.generate(1)[0],True)["tx"] + mined = mining_node.getblock(mining_node.generate(1)[0], True)["tx"] self.sync_blocks(self.nodes[0:3], wait=.1) # update which txouts are confirmed newmem = [] @@ -191,10 +172,6 @@ def run_test(self): 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 = [] @@ -210,13 +187,13 @@ def run_test(self): # Use txouts to monitor the available utxo, since these won't be tracked in wallet reps = 0 while (reps < 5): - #Double txouts to txouts2 - while (len(self.txouts)>0): + # Double txouts to txouts2 + while (len(self.txouts) > 0): split_inputs(self.nodes[0], self.txouts, self.txouts2) while (len(self.nodes[0].getrawmempool()) > 0): self.nodes[0].generate(1) - #Double txouts2 to txouts - while (len(self.txouts2)>0): + # Double txouts2 to txouts + while (len(self.txouts2) > 0): split_inputs(self.nodes[0], self.txouts2, self.txouts) while (len(self.nodes[0].getrawmempool()) > 0): self.nodes[0].generate(1) @@ -235,7 +212,7 @@ def run_test(self): self.fees_per_kb = [] self.memutxo = [] - self.confutxo = self.txouts # Start with the set of confirmed txouts after splitting + self.confutxo = self.txouts # Start with the set of confirmed txouts after splitting self.log.info("Will output estimates for 1/2/3/6/15/25 blocks") for i in range(2):