diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -3,4 +3,4 @@ This release includes the following features and fixes: - + - Deprecated the `-reserveChangeKey` option for `fundrawtransaction` wallet rpc command. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3466,7 +3466,7 @@ " \"lockUnspents\" (boolean, optional, default " "false) Lock selected unspent outputs\n" " \"reserveChangeKey\" (boolean, optional, default true) " - "Reserves the change output key from the keypool\n" + "DEPRECATED. Reserves the change output key from the keypool\n" " \"feeRate\" (numeric, optional, default not " "set: makes wallet determine the fee) Set a specific feerate (" + CURRENCY_UNIT + @@ -3517,6 +3517,7 @@ CCoinControl coinControl; int changePosition = -1; bool lockUnspents = false; + // DEPRECATED, should be removed in 0.20 bool reserveChangeKey = true; UniValue subtractFeeFromOutputs; std::set setSubtractFeeFromOutputs; @@ -3537,6 +3538,7 @@ {"changePosition", UniValueType(UniValue::VNUM)}, {"includeWatching", UniValueType(UniValue::VBOOL)}, {"lockUnspents", UniValueType(UniValue::VBOOL)}, + // DEPRECATED, should be removed in 0.20 {"reserveChangeKey", UniValueType(UniValue::VBOOL)}, // will be checked below {"feeRate", UniValueType()}, @@ -3571,8 +3573,10 @@ lockUnspents = options["lockUnspents"].get_bool(); } - if (options.exists("reserveChangeKey")) { - reserveChangeKey = options["reserveChangeKey"].get_bool(); + if (IsDeprecatedRPCEnabled(gArgs, "fundrawtransaction")) { + if (options.exists("reserveChangeKey")) { + reserveChangeKey = options["reserveChangeKey"].get_bool(); + } } if (options.exists("feeRate")) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -988,7 +988,8 @@ int &nChangePosInOut, std::string &strFailReason, bool lockUnspents, const std::set &setSubtractFeeFromOutputs, - CCoinControl coinControl, bool keepReserveKey = true); + CCoinControl coinControl, + bool reserveChangeKey = true); bool SignTransaction(CMutableTransaction &tx); /** diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -19,6 +19,7 @@ #include "policy/policy.h" #include "primitives/block.h" #include "primitives/transaction.h" +#include "rpc/server.h" #include "scheduler.h" #include "script/script.h" #include "script/sighashtype.h" @@ -2788,6 +2789,12 @@ if (nChangePosInOut != -1) { tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]); + if (!IsDeprecatedRPCEnabled(gArgs, "fundrawtransaction")) { + // we dont have the normal Create/Commit cycle, and dont want to + // risk reusing change, so just remove the key from the keypool + // here. + reservekey.KeepKey(); + } } // Copy output sizes from new transaction; they may have had the fee @@ -2808,9 +2815,12 @@ } } - // Optionally keep the change output key. - if (keepReserveKey) { - reservekey.KeepKey(); + // DEPRECATED, remove in 0.20 with -reserveChangeKey + if (IsDeprecatedRPCEnabled(gArgs, "fundrawtransaction")) { + // Optionally keep the change output key. + if (keepReserveKey) { + reservekey.KeepKey(); + } } return true; diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -27,8 +27,10 @@ class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): - self.num_nodes = 4 + self.num_nodes = 5 self.setup_clean_chain = True + self.extra_args = [[], [], [], [], + ["-deprecatedrpc=fundrawtransaction"]] def setup_network(self, split=False): self.setup_nodes() @@ -37,6 +39,7 @@ connect_nodes_bi(self.nodes[1], self.nodes[2]) connect_nodes_bi(self.nodes[0], self.nodes[2]) connect_nodes_bi(self.nodes[0], self.nodes[3]) + connect_nodes_bi(self.nodes[0], self.nodes[4]) def run_test(self): min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] @@ -74,6 +77,7 @@ watchonly_address, watchonly_amount) self.nodes[0].sendtoaddress( self.nodes[3].getnewaddress(), watchonly_amount / 10) + self.nodes[0].sendtoaddress(self.nodes[4].getnewaddress(), 5.0) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.5) self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0) @@ -476,6 +480,7 @@ self.nodes[1].node_encrypt_wallet("test") self.stop_node(2) self.stop_node(3) + self.stop_node(4) self.start_nodes() # This test is not meant to test fee estimation and we'd like @@ -487,6 +492,7 @@ connect_nodes_bi(self.nodes[1], self.nodes[2]) connect_nodes_bi(self.nodes[0], self.nodes[2]) connect_nodes_bi(self.nodes[0], self.nodes[3]) + connect_nodes_bi(self.nodes[0], self.nodes[4]) self.sync_all() # drain the keypool @@ -681,21 +687,27 @@ result3['fee'], FromHex(CTransaction(), result3['hex']).billable_size(), 10 * result_fee_rate, 10) # - # Test address reuse option # + # Test address reuse option (DEPRECATED)# # - - result3 = self.nodes[3].fundrawtransaction( - rawTx, {"reserveChangeKey": False}) - res_dec = self.nodes[0].decoderawtransaction(result3["hex"]) + dInputs = [] + dOutputs = {self.nodes[4].getnewaddress(): 1} + dRawTx = self.nodes[4].createrawtransaction(dInputs, dOutputs) + dResult = self.nodes[4].fundrawtransaction( + dRawTx, {"reserveChangeKey": False}) + res_dec = self.nodes[0].decoderawtransaction(dResult["hex"]) changeaddress = "" for out in res_dec['vout']: if out['value'] > 1.0: changeaddress += out['scriptPubKey']['addresses'][0] assert(changeaddress != "") - nextaddr = self.nodes[3].getrawchangeaddress() + nextaddr = self.nodes[4].getrawchangeaddress() # frt should not have removed the key from the keypool assert(changeaddress == nextaddr) + # + # Test no address reuse occurs # + # + result3 = self.nodes[3].fundrawtransaction(rawTx) res_dec = self.nodes[0].decoderawtransaction(result3["hex"]) changeaddress = ""