Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RPC] add feerate option to fundrawtransaction #7967

Merged
merged 2 commits into from Jun 3, 2016
Merged

[RPC] add feerate option to fundrawtransaction #7967

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b35e48
[RPC] add feerate option to fundrawtransaction
jonasschnelli Apr 28, 2016
04eaa90
Add more clear interface for CoinControl.h regarding individual feerate
jonasschnelli May 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions qa/rpc-tests/fundrawtransaction.py
View file
Open in desktop
Expand Up @@ -677,6 +677,14 @@ def run_test(self):
assert(signedtx["complete"])
self.nodes[0].sendrawtransaction(signedtx["hex"])

inputs = []
outputs = {self.nodes[2].getnewaddress() : 1}
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
result = self.nodes[3].fundrawtransaction(rawtx, )
Copy link
Member

@maflcko maflcko Jun 3, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: empty arg.
Nit: Missing comment mentioning that min_relay_tx_fee=1000 is used.

(Comments help to identify the issue when something breaks)

result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2000})
result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10000})
assert_equal(result['fee']*2, result2['fee'])
assert_equal(result['fee']*10, result3['fee'])

if __name__ == '__main__':
RawTransactionsTest().main()
3 changes: 3 additions & 0 deletions src/coincontrol.h
View file
Open in desktop
Expand Up @@ -18,6 +18,8 @@ class CCoinControl
bool fAllowWatchOnly;
//! Minimum absolute fee (not per kilobyte)
CAmount nMinimumTotalFee;
//! Feerate to use (0 = estimate fee with payTxFee fallback)
CFeeRate nFeeRate;

CCoinControl()
{
Expand All @@ -31,6 +33,7 @@ class CCoinControl
fAllowWatchOnly = false;
setSelected.clear();
nMinimumTotalFee = 0;
nFeeRate = CFeeRate(0);
}

bool HasSelected() const
Expand Down
13 changes: 9 additions & 4 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Expand Up @@ -2458,6 +2458,7 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
" \"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"
" \"feeRate\" (numeric, optional, default 0=estimate) Set a specific feerate (fee per KB)\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default is not "estimate" but whatever happens to be set by the user on startup or after startup.

Either make it default 0 or default not set.

" }\n"
" for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}\n"
"\nResult:\n"
Expand All @@ -2484,6 +2485,7 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
int changePosition = -1;
bool includeWatching = false;
bool lockUnspents = false;
CFeeRate feeRate = CFeeRate(0);

if (params.size() > 1) {
if (params[1].type() == UniValue::VBOOL) {
Expand All @@ -2495,7 +2497,7 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)

UniValue options = params[1];

RPCTypeCheckObj(options, boost::assign::map_list_of("changeAddress", UniValue::VSTR)("changePosition", UniValue::VNUM)("includeWatching", UniValue::VBOOL)("lockUnspents", UniValue::VBOOL), true, true);
RPCTypeCheckObj(options, boost::assign::map_list_of("changeAddress", UniValue::VSTR)("changePosition", UniValue::VNUM)("includeWatching", UniValue::VBOOL)("lockUnspents", UniValue::VBOOL)("feeRate", UniValue::VNUM), true, true);

if (options.exists("changeAddress")) {
CBitcoinAddress address(options["changeAddress"].get_str());
Expand All @@ -2514,6 +2516,9 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)

if (options.exists("lockUnspents"))
lockUnspents = options["lockUnspents"].get_bool();

if (options.exists("feeRate"))
feeRate = CFeeRate(options["feeRate"].get_real());
}
}

Expand All @@ -2529,16 +2534,16 @@ UniValue fundrawtransaction(const UniValue& params, bool fHelp)
throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds");

CMutableTransaction tx(origTx);
CAmount nFee;
CAmount nFeeOut;
string strFailReason;

if(!pwalletMain->FundTransaction(tx, nFee, changePosition, strFailReason, includeWatching, lockUnspents, changeAddress))
if(!pwalletMain->FundTransaction(tx, nFeeOut, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, changeAddress))
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);

UniValue result(UniValue::VOBJ);
result.push_back(Pair("hex", EncodeHexTx(tx)));
result.push_back(Pair("changepos", changePosition));
result.push_back(Pair("fee", ValueFromAmount(nFee)));
result.push_back(Pair("fee", ValueFromAmount(nFeeOut)));

return result;
}
Expand Down
6 changes: 5 additions & 1 deletion src/wallet/wallet.cpp
View file
Open in desktop
Expand Up @@ -1918,7 +1918,7 @@ bool CWallet::SelectCoins(const vector<COutput>& vAvailableCoins, const CAmount&
return res;
}

bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange)
bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange)
{
vector<CRecipient> vecSend;

Expand All @@ -1933,6 +1933,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
coinControl.destChange = destChange;
coinControl.fAllowOtherInputs = true;
coinControl.fAllowWatchOnly = includeWatching;
coinControl.nFeeRate = specificFeeRate;

BOOST_FOREACH(const CTxIn& txin, tx.vin)
coinControl.Select(txin.prevout);

Expand Down Expand Up @@ -2242,6 +2244,8 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) {
nFeeNeeded = coinControl->nMinimumTotalFee;
}
if (coinControl && coinControl->nFeeRate > CFeeRate(0))
nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK.
Though should we be using '0' as a special marker value? What if you want to override it to 0? I think I'd prefer an explicit boolean 'overrideFeeRate'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We could use an extra boolean (like overrideFeeRate). My thoughts where that a CFeeRate(0) is not practical but I agree, it would technically be possible to use a zero feerate. I'll add the bool.


// If we made it here and we aren't even able to meet the relay fee on the next pass, give up
// because we must be at the maximum allowed fee.
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
View file
Open in desktop
Expand Up @@ -740,7 +740,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
* Insert additional inputs into the transaction by
* calling CreateTransaction();
*/
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange = CNoDestination());
bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, const CFeeRate& specificFeeRate, int& nChangePosInOut, std::string& strFailReason, bool includeWatching, bool lockUnspents, const CTxDestination& destChange = CNoDestination());

/**
* Create a new transaction paying the recipients with a set of coins
Expand Down