Skip to content

Commit be14ca5

Browse files
laanwjlaanwj
committed
Merge #7518: Add multiple options to fundrawtransaction
f2d0944 Add lockUnspents option to fundrawtransaction (João Barbosa) af4fe7f Add change options to fundrawtransaction (João Barbosa) 41e835d Add strict flag to RPCTypeCheckObj (João Barbosa)
2 parents a4ca44d + f2d0944 commit be14ca5

File tree

6 files changed

+183
-28
lines changed

6 files changed

+183
-28
lines changed

‎qa/rpc-tests/fundrawtransaction.py‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,83 @@ def run_test(self):
177177
assert_equal(fee + totalOut, utx['amount']) #compare vin total and totalout+fee
178178

179179

180+
####################################################
181+
# test a fundrawtransaction with an invalid option #
182+
####################################################
183+
utx = False
184+
listunspent = self.nodes[2].listunspent()
185+
for aUtx in listunspent:
186+
if aUtx['amount'] == 5.0:
187+
utx = aUtx
188+
break
189+
190+
assert_equal(utx!=False, True)
191+
192+
inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']} ]
193+
outputs = { self.nodes[0].getnewaddress() : Decimal(4.0) }
194+
rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
195+
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
196+
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
197+
198+
try:
199+
self.nodes[2].fundrawtransaction(rawtx, {'foo': 'bar'})
200+
raise AssertionError("Accepted invalid option foo")
201+
except JSONRPCException,e:
202+
assert("Unexpected key foo" in e.error['message'])
203+
204+
205+
############################################################
206+
# test a fundrawtransaction with an invalid change address #
207+
############################################################
208+
utx = False
209+
listunspent = self.nodes[2].listunspent()
210+
for aUtx in listunspent:
211+
if aUtx['amount'] == 5.0:
212+
utx = aUtx
213+
break
214+
215+
assert_equal(utx!=False, True)
216+
217+
inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']} ]
218+
outputs = { self.nodes[0].getnewaddress() : Decimal(4.0) }
219+
rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
220+
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
221+
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
222+
223+
try:
224+
self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': 'foobar'})
225+
raise AssertionError("Accepted invalid bitcoin address")
226+
except JSONRPCException,e:
227+
assert("changeAddress must be a valid bitcoin address" in e.error['message'])
228+
229+
230+
231+
############################################################
232+
# test a fundrawtransaction with a provided change address #
233+
############################################################
234+
utx = False
235+
listunspent = self.nodes[2].listunspent()
236+
for aUtx in listunspent:
237+
if aUtx['amount'] == 5.0:
238+
utx = aUtx
239+
break
240+
241+
assert_equal(utx!=False, True)
242+
243+
inputs = [ {'txid' : utx['txid'], 'vout' : utx['vout']} ]
244+
outputs = { self.nodes[0].getnewaddress() : Decimal(4.0) }
245+
rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
246+
dec_tx = self.nodes[2].decoderawtransaction(rawtx)
247+
assert_equal(utx['txid'], dec_tx['vin'][0]['txid'])
248+
249+
change = self.nodes[2].getnewaddress()
250+
rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': change, 'changePosition': 0})
251+
dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
252+
out = dec_tx['vout'][0];
253+
assert_equal(change, out['scriptPubKey']['addresses'][0])
254+
255+
256+
180257
#########################################################################
181258
# test a fundrawtransaction with a VIN smaller than the required amount #
182259
#########################################################################
@@ -568,7 +645,7 @@ def run_test(self):
568645
outputs = {self.nodes[2].getnewaddress() : watchonly_amount / 2}
569646
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
570647

571-
result = self.nodes[3].fundrawtransaction(rawtx, True)
648+
result = self.nodes[3].fundrawtransaction(rawtx, {'includeWatching': True })
572649
res_dec = self.nodes[0].decoderawtransaction(result["hex"])
573650
assert_equal(len(res_dec["vin"]), 1)
574651
assert_equal(res_dec["vin"][0]["txid"], watchonly_txid)
@@ -584,6 +661,7 @@ def run_test(self):
584661
outputs = {self.nodes[2].getnewaddress() : watchonly_amount}
585662
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
586663

664+
# Backward compatibility test (2nd param is includeWatching)
587665
result = self.nodes[3].fundrawtransaction(rawtx, True)
588666
res_dec = self.nodes[0].decoderawtransaction(result["hex"])
589667
assert_equal(len(res_dec["vin"]), 2)

‎src/rpc/server.cpp‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ void RPCTypeCheck(const UniValue& params,
8989

9090
void RPCTypeCheckObj(const UniValue& o,
9191
const map<string, UniValue::VType>& typesExpected,
92-
bool fAllowNull)
92+
bool fAllowNull,
93+
bool fStrict)
9394
{
9495
BOOST_FOREACH(const PAIRTYPE(string, UniValue::VType)& t, typesExpected)
9596
{
@@ -104,6 +105,18 @@ void RPCTypeCheckObj(const UniValue& o,
104105
throw JSONRPCError(RPC_TYPE_ERROR, err);
105106
}
106107
}
108+
109+
if (fStrict)
110+
{
111+
BOOST_FOREACH(const string& k, o.getKeys())
112+
{
113+
if (typesExpected.count(k) == 0)
114+
{
115+
string err = strprintf("Unexpected key %s", k);
116+
throw JSONRPCError(RPC_TYPE_ERROR, err);
117+
}
118+
}
119+
}
107120
}
108121

109122
CAmount AmountFromValue(const UniValue& value)

‎src/rpc/server.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ void RPCTypeCheck(const UniValue& params,
7070
Use like: RPCTypeCheckObj(object, boost::assign::map_list_of("name", str_type)("value", int_type));
7171
*/
7272
void RPCTypeCheckObj(const UniValue& o,
73-
const std::map<std::string, UniValue::VType>& typesExpected, bool fAllowNull=false);
73+
const std::map<std::string, UniValue::VType>& typesExpected, bool fAllowNull=false, bool fStrict=false);
7474

7575
/** Opaque base class for timers returned by NewTimerFunc.
7676
* This provides no methods at the moment, but makes sure that delete

‎src/wallet/rpcwallet.cpp‎

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2437,7 +2437,7 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
24372437

24382438
if (fHelp || params.size() < 1 || params.size() > 2)
24392439
throw runtime_error(
2440-
"fundrawtransaction \"hexstring\" includeWatching\n"
2440+
"fundrawtransaction \"hexstring\" ( options )\n"
24412441
"\nAdd inputs to a transaction until it has enough in value to meet its out value.\n"
24422442
"This will not modify existing inputs, and will add one change output to the outputs.\n"
24432443
"Note that inputs which were signed may need to be resigned after completion since in/outputs have been added.\n"
@@ -2447,8 +2447,15 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
24472447
"in the wallet using importaddress or addmultisigaddress (to calculate fees).\n"
24482448
"Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n"
24492449
"\nArguments:\n"
2450-
"1. \"hexstring\" (string, required) The hex string of the raw transaction\n"
2451-
"2. includeWatching (boolean, optional, default false) Also select inputs which are watch only\n"
2450+
"1. \"hexstring\" (string, required) The hex string of the raw transaction\n"
2451+
"2. options (object, optional)\n"
2452+
" {\n"
2453+
" \"changeAddress\" (string, optional, default pool address) The bitcoin address to receive the change\n"
2454+
" \"changePosition\" (numeric, optional, default random) The index of the change output\n"
2455+
" \"includeWatching\" (boolean, optional, default false) Also select inputs which are watch only\n"
2456+
" \"lockUnspents\" (boolean, optional, default false) Lock selected unspent outputs\n"
2457+
" }\n"
2458+
" for backward compatibility: passing in a true instzead of an object will result in {\"includeWatching\":true}\n"
24522459
"\nResult:\n"
24532460
"{\n"
24542461
" \"hex\": \"value\", (string) The resulting raw transaction (hex-encoded string)\n"
@@ -2467,7 +2474,44 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
24672474
+ HelpExampleCli("sendrawtransaction", "\"signedtransactionhex\"")
24682475
);
24692476

2470-
RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VBOOL));
2477+
RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR));
2478+
2479+
CTxDestination changeAddress = CNoDestination();
2480+
int changePosition = -1;
2481+
bool includeWatching = false;
2482+
bool lockUnspents = false;
2483+
2484+
if (params.size() > 1) {
2485+
if (params[1].type() == UniValue::VBOOL) {
2486+
// backward compatibility bool only fallback
2487+
includeWatching = params[1].get_bool();
2488+
}
2489+
else {
2490+
RPCTypeCheck(params, boost::assign::list_of(UniValue::VSTR)(UniValue::VOBJ));
2491+
2492+
UniValue options = params[1];
2493+
2494+
RPCTypeCheckObj(options, boost::assign::map_list_of("changeAddress", UniValue::VSTR)("changePosition", UniValue::VNUM)("includeWatching", UniValue::VBOOL)("lockUnspents", UniValue::VBOOL), true, true);
2495+
2496+
if (options.exists("changeAddress")) {
2497+
CBitcoinAddress address(options["changeAddress"].get_str());
2498+
2499+
if (!address.IsValid())
2500+
throw JSONRPCError(RPC_INVALID_PARAMETER, "changeAddress must be a valid bitcoin address");
2501+
2502+
changeAddress = address.Get();
2503+
}
2504+
2505+
if (options.exists("changePosition"))
2506+
changePosition = options["changePosition"].get_int();
2507+
2508+
if (options.exists("includeWatching"))
2509+
includeWatching = options["includeWatching"].get_bool();
2510+
2511+
if (options.exists("lockUnspents"))
2512+
lockUnspents = options["lockUnspents"].get_bool();
2513+
}
2514+
}
24712515

24722516
// parse hex string from parameter
24732517
CTransaction origTx;
@@ -2477,20 +2521,19 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
24772521
if (origTx.vout.size() == 0)
24782522
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");
24792523

2480-
bool includeWatching = false;
2481-
if (params.size() > 1)
2482-
includeWatching = params[1].get_bool();
2524+
if (changePosition != -1 && (changePosition < 0 || changePosition > origTx.vout.size()))
2525+
throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds");
24832526

24842527
CMutableTransaction tx(origTx);
24852528
CAmount nFee;
24862529
string strFailReason;
2487-
int nChangePos = -1;
2488-
if(!pwalletMain->FundTransaction(tx, nFee, nChangePos, strFailReason, includeWatching))
2530+
2531+
if(!pwalletMain->FundTransaction(tx, nFee, changePosition, strFailReason, includeWatching, lockUnspents, changeAddress))
24892532
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
24902533

24912534
UniValue result(UniValue::VOBJ);
24922535
result.push_back(Pair("hex", EncodeHexTx(tx)));
2493-
result.push_back(Pair("changepos", nChangePos));
2536+
result.push_back(Pair("changepos", changePosition));
24942537
result.push_back(Pair("fee", ValueFromAmount(nFee)));
24952538

24962539
return result;

‎src/wallet/wallet.cpp‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,7 @@ bool CWallet::SelectCoins(const vector<COutput>& vAvailableCoins, const CAmount&
19321932
return res;
19331933
}
19341934

1935-
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount &nFeeRet, int& nChangePosRet, std::string& strFailReason, bool includeWatching)
1935+
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange)
19361936
{
19371937
vector<CRecipient> vecSend;
19381938

@@ -1944,33 +1944,43 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount &nFeeRet, int& nC
19441944
}
19451945

19461946
CCoinControl coinControl;
1947+
coinControl.destChange = destChange;
19471948
coinControl.fAllowOtherInputs = true;
19481949
coinControl.fAllowWatchOnly = includeWatching;
19491950
BOOST_FOREACH(const CTxIn& txin, tx.vin)
19501951
coinControl.Select(txin.prevout);
19511952

19521953
CReserveKey reservekey(this);
19531954
CWalletTx wtx;
1954-
if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosRet, strFailReason, &coinControl, false))
1955+
if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false))
19551956
return false;
19561957

1957-
if (nChangePosRet != -1)
1958-
tx.vout.insert(tx.vout.begin() + nChangePosRet, wtx.vout[nChangePosRet]);
1958+
if (nChangePosInOut != -1)
1959+
tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.vout[nChangePosInOut]);
19591960

19601961
// Add new txins (keeping original txin scriptSig/order)
19611962
BOOST_FOREACH(const CTxIn& txin, wtx.vin)
19621963
{
19631964
if (!coinControl.IsSelected(txin.prevout))
1965+
{
19641966
tx.vin.push_back(txin);
1967+
1968+
if (lockUnspents)
1969+
{
1970+
LOCK2(cs_main, cs_wallet);
1971+
LockCoin(txin.prevout);
1972+
}
1973+
}
19651974
}
19661975

19671976
return true;
19681977
}
19691978

19701979
bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet,
1971-
int& nChangePosRet, std::string& strFailReason, const CCoinControl* coinControl, bool sign)
1980+
int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign)
19721981
{
19731982
CAmount nValue = 0;
1983+
int nChangePosRequest = nChangePosInOut;
19741984
unsigned int nSubtractFeeFromAmount = 0;
19751985
BOOST_FOREACH (const CRecipient& recipient, vecSend)
19761986
{
@@ -2036,10 +2046,10 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
20362046
// Start with no fee and loop until there is enough fee
20372047
while (true)
20382048
{
2049+
nChangePosInOut = nChangePosRequest;
20392050
txNew.vin.clear();
20402051
txNew.vout.clear();
20412052
wtxNew.fFromMe = true;
2042-
nChangePosRet = -1;
20432053
bool fFirst = true;
20442054

20452055
CAmount nValueToSelect = nValue;
@@ -2159,14 +2169,24 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
21592169
// add the dust to the fee.
21602170
if (newTxOut.IsDust(::minRelayTxFee))
21612171
{
2172+
nChangePosInOut = -1;
21622173
nFeeRet += nChange;
21632174
reservekey.ReturnKey();
21642175
}
21652176
else
21662177
{
2167-
// Insert change txn at random position:
2168-
nChangePosRet = GetRandInt(txNew.vout.size()+1);
2169-
vector<CTxOut>::iterator position = txNew.vout.begin()+nChangePosRet;
2178+
if (nChangePosInOut == -1)
2179+
{
2180+
// Insert change txn at random position:
2181+
nChangePosInOut = GetRandInt(txNew.vout.size()+1);
2182+
}
2183+
else if (nChangePosInOut > txNew.vout.size())
2184+
{
2185+
strFailReason = _("Change index out of range");
2186+
return false;
2187+
}
2188+
2189+
vector<CTxOut>::iterator position = txNew.vout.begin()+nChangePosInOut;
21702190
txNew.vout.insert(position, newTxOut);
21712191
}
21722192
}
@@ -2842,13 +2862,13 @@ void CWallet::GetScriptForMining(boost::shared_ptr<CReserveScript> &script)
28422862
script->reserveScript = CScript() << ToByteVector(pubkey) << OP_CHECKSIG;
28432863
}
28442864

2845-
void CWallet::LockCoin(COutPoint& output)
2865+
void CWallet::LockCoin(const COutPoint& output)
28462866
{
28472867
AssertLockHeld(cs_wallet); // setLockedCoins
28482868
setLockedCoins.insert(output);
28492869
}
28502870

2851-
void CWallet::UnlockCoin(COutPoint& output)
2871+
void CWallet::UnlockCoin(const COutPoint& output)
28522872
{
28532873
AssertLockHeld(cs_wallet); // setLockedCoins
28542874
setLockedCoins.erase(output);

‎src/wallet/wallet.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -667,8 +667,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
667667
bool IsSpent(const uint256& hash, unsigned int n) const;
668668

669669
bool IsLockedCoin(uint256 hash, unsigned int n) const;
670-
void LockCoin(COutPoint& output);
671-
void UnlockCoin(COutPoint& output);
670+
void LockCoin(const COutPoint& output);
671+
void UnlockCoin(const COutPoint& output);
672672
void UnlockAllCoins();
673673
void ListLockedCoins(std::vector<COutPoint>& vOutpts);
674674

@@ -739,13 +739,14 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
739739
* Insert additional inputs into the transaction by
740740
* calling CreateTransaction();
741741
*/
742-
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosRet, std::string& strFailReason, bool includeWatching);
742+
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange = CNoDestination());
743743

744744
/**
745745
* Create a new transaction paying the recipients with a set of coins
746746
* selected by SelectCoins(); Also create the change output, when needed
747+
* @note passing nChangePosInOut as -1 will result in setting a random position
747748
*/
748-
bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosRet,
749+
bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut,
749750
std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true);
750751
bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey);
751752

0 commit comments

Comments
 (0)