LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #9377: fundrawtransaction: Keep change-output keys by default, make it optional

Browse source 
c9f3062 Add fundrawtransactions new reserveChangeKey option to the release notes (Jonas Schnelli)
9eb325d [QA] Add test for fundrawtransactions new reserveChangeKey option (Jonas Schnelli)
9aa4e6a [Wallet] Add an option to keep the change address key, true by default (Jonas Schnelli)
This commit is contained in:
Wladimir J. van der Laan 2017-01-20 14:32:09 +01:00
commit fb75cd04bb
No known key found for this signature in database
GPG key ID: 74810B012346C9A6
5 changed files with 50 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
doc/release-notes.md
View file

@ -128,6 +128,16 @@ Call "getmininginfo" loses the "testnet" field in favor of the more generic "cha
### Wallet
0.14.0 Fundrawtransaction change address reuse
==============================================
Before 0.14, `fundrawtransaction` was by default wallet stateless. In almost all cases `fundrawtransaction` does add a change-output to the outputs of the funded transaction. Before 0.14, the used keypool key was never marked as change-address key and directly returned to the keypool (leading to address reuse).
Before 0.14, calling `getnewaddress` directly after `fundrawtransaction` did generate the same address as the change-output address.
Since 0.14, fundrawtransaction does reserve the change-output-key from the keypool by default (optional by setting `reserveChangeKey`, default = `true`)
Users should also consider using `getrawchangeaddress()` in conjunction with `fundrawtransaction`'s `changeAddress` option.
### GUI
### Tests

28
qa/rpc-tests/fundrawtransaction.py
View file

@ -651,7 +651,7 @@ class RawTransactionsTest(BitcoinTestFramework):
assert_equal(len(self.nodes[3].listunspent(1)), 1)
inputs = []
outputs = {self.nodes[2].getnewaddress() : 1}
outputs = {self.nodes[3].getnewaddress() : 1}
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee)
result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee})
@ -660,6 +660,32 @@ class RawTransactionsTest(BitcoinTestFramework):
assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)
#############################
# Test address reuse option #
#############################
result3 = self.nodes[3].fundrawtransaction(rawtx, {"reserveChangeKey": False})
res_dec = self.nodes[0].decoderawtransaction(result3["hex"])
changeaddress = ""
for out in res_dec['vout']:
if out['value'] > 1.0:
changeaddress += out['scriptPubKey']['addresses'][0]
assert(changeaddress != "")
nextaddr = self.nodes[3].getnewaddress()
# frt should not have removed the key from the keypool
assert(changeaddress == nextaddr)
result3 = self.nodes[3].fundrawtransaction(rawtx)
res_dec = self.nodes[0].decoderawtransaction(result3["hex"])
changeaddress = ""
for out in res_dec['vout']:
if out['value'] > 1.0:
changeaddress += out['scriptPubKey']['addresses'][0]
assert(changeaddress != "")
nextaddr = self.nodes[3].getnewaddress()
# Now the change address key should be removed from the keypool
assert(changeaddress != nextaddr)
######################################
# Test subtractFeeFromOutputs option #
######################################

8
src/wallet/rpcwallet.cpp
View file

@ -2511,6 +2511,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
" \"changePosition\" (numeric, optional, default random) The index of the change output\n"
" \"includeWatching\" (boolean, optional, default false) Also select inputs which are watch only\n"
" \"lockUnspents\" (boolean, optional, default false) Lock selected unspent outputs\n"
" \"reserveChangeKey\" (boolean, optional, default true) 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 + " per KB)\n"
" \"subtractFeeFromOutputs\" (array, optional) A json array of integers.\n"
" The fee will be equally deducted from the amount of each specified output.\n"
@ -2543,6 +2544,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
int changePosition = -1;
bool includeWatching = false;
bool lockUnspents = false;
bool reserveChangeKey = true;
CFeeRate feeRate = CFeeRate(0);
bool overrideEstimatedFeerate = false;
UniValue subtractFeeFromOutputs;
@ -2564,6 +2566,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
{"changePosition", UniValueType(UniValue::VNUM)},
{"includeWatching", UniValueType(UniValue::VBOOL)},
{"lockUnspents", UniValueType(UniValue::VBOOL)},
{"reserveChangeKey", UniValueType(UniValue::VBOOL)},
{"feeRate", UniValueType()}, // will be checked below
{"subtractFeeFromOutputs", UniValueType(UniValue::VARR)},
},
@ -2587,6 +2590,9 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
if (options.exists("lockUnspents"))
lockUnspents = options["lockUnspents"].get_bool();
if (options.exists("reserveChangeKey"))
reserveChangeKey = options["reserveChangeKey"].get_bool();
if (options.exists("feeRate"))
{
feeRate = CFeeRate(AmountFromValue(options["feeRate"]));
@ -2623,7 +2629,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
CAmount nFeeOut;
string strFailReason;
if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, setSubtractFeeFromOutputs, changeAddress))
if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, setSubtractFeeFromOutputs, reserveChangeKey, changeAddress))
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
UniValue result(UniValue::VOBJ);

6
src/wallet/wallet.cpp
View file

@ -2282,7 +2282,7 @@ bool CWallet::SelectCoins(const vector<COutput>& vAvailableCoins, const CAmount&
return res;
}
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool overrideEstimatedFeeRate, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, const CTxDestination& destChange)
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool overrideEstimatedFeeRate, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, bool keepReserveKey, const CTxDestination& destChange)
{
vector<CRecipient> vecSend;
@ -2331,6 +2331,10 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
}
}
// optionally keep the change output key
if (keepReserveKey)
reservekey.KeepKey();
return true;
}

2
src/wallet/wallet.h
View file

@ -780,7 +780,7 @@ public:
* Insert additional inputs into the transaction by
* calling CreateTransaction();
*/
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool overrideEstimatedFeeRate, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, const CTxDestination& destChange = CNoDestination());
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool overrideEstimatedFeeRate, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, bool keepReserveKey = true, const CTxDestination& destChange = CNoDestination());
/**
* Create a new transaction paying the recipients with a set of coins