Add 'subtractFeeFromOutputs' option to 'fundrawtransaction'.

This commit is contained in:
Chris Moore 2016-12-13 13:36:23 -08:00
parent 26fe5c98ab
commit 453bda63dd
5 changed files with 117 additions and 14 deletions

View file

@ -660,5 +660,75 @@ 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 subtractFeeFromOutputs option #
######################################
# Make sure there is exactly one input so coin selection can't skew the result
assert_equal(len(self.nodes[3].listunspent(1)), 1)
inputs = []
outputs = {self.nodes[2].getnewaddress(): 1}
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
result = [self.nodes[3].fundrawtransaction(rawtx), # uses min_relay_tx_fee (set by settxfee)
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses min_relay_tx_fee (set by settxfee)
self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee}),
self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee, "subtractFeeFromOutputs": [0]})]
dec_tx = [self.nodes[3].decoderawtransaction(tx['hex']) for tx in result]
output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)]
change = [d['vout'][r['changepos']]['value'] for d, r in zip(dec_tx, result)]
assert_equal(result[0]['fee'], result[1]['fee'], result[2]['fee'])
assert_equal(result[3]['fee'], result[4]['fee'])
assert_equal(change[0], change[1])
assert_equal(output[0], output[1])
assert_equal(output[0], output[2] + result[2]['fee'])
assert_equal(change[0] + result[0]['fee'], change[2])
assert_equal(output[3], output[4] + result[4]['fee'])
assert_equal(change[3] + result[3]['fee'], change[4])
inputs = []
outputs = {self.nodes[2].getnewaddress(): value for value in (1.0, 1.1, 1.2, 1.3)}
keys = list(outputs.keys())
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
result = [self.nodes[3].fundrawtransaction(rawtx),
# split the fee between outputs 0, 2, and 3, but not output 1
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0, 2, 3]})]
dec_tx = [self.nodes[3].decoderawtransaction(result[0]['hex']),
self.nodes[3].decoderawtransaction(result[1]['hex'])]
# Nested list of non-change output amounts for each transaction
output = [[out['value'] for i, out in enumerate(d['vout']) if i != r['changepos']]
for d, r in zip(dec_tx, result)]
# List of differences in output amounts between normal and subtractFee transactions
share = [o0 - o1 for o0, o1 in zip(output[0], output[1])]
# output 1 is the same in both transactions
assert_equal(share[1], 0)
# the other 3 outputs are smaller as a result of subtractFeeFromOutputs
assert_greater_than(share[0], 0)
assert_greater_than(share[2], 0)
assert_greater_than(share[3], 0)
# outputs 2 and 3 take the same share of the fee
assert_equal(share[2], share[3])
# output 0 takes at least as much share of the fee, and no more than 2 satoshis more, than outputs 2 and 3
assert_greater_than_or_equal(share[0], share[2])
assert_greater_than_or_equal(share[2] + Decimal(2e-8), share[0])
# the fee is the same in both transactions
assert_equal(result[0]['fee'], result[1]['fee'])
# the total subtracted from the outputs is equal to the fee
assert_equal(share[0] + share[2] + share[3], result[0]['fee'])
if __name__ == '__main__':
RawTransactionsTest().main()

View file

@ -524,14 +524,18 @@ def assert_fee_amount(fee, tx_size, fee_per_kB):
if fee > (tx_size + 2) * fee_per_kB / 1000:
raise AssertionError("Fee of %s BTC too high! (Should be %s BTC)"%(str(fee), str(target_fee)))
def assert_equal(thing1, thing2):
if thing1 != thing2:
raise AssertionError("%s != %s"%(str(thing1),str(thing2)))
def assert_equal(thing1, thing2, *args):
if thing1 != thing2 or any(thing1 != arg for arg in args):
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
def assert_greater_than(thing1, thing2):
if thing1 <= thing2:
raise AssertionError("%s <= %s"%(str(thing1),str(thing2)))
def assert_greater_than_or_equal(thing1, thing2):
if thing1 < thing2:
raise AssertionError("%s < %s"%(str(thing1),str(thing2)))
def assert_raises(exc, fun, *args, **kwds):
assert_raises_message(exc, None, fun, *args, **kwds)

View file

@ -2464,7 +2464,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
throw runtime_error(
"fundrawtransaction \"hexstring\" ( options )\n"
"\nAdd inputs to a transaction until it has enough in value to meet its out value.\n"
"This will not modify existing inputs, and will add one change output to the outputs.\n"
"This will not modify existing inputs, and will add at most one change output to the outputs.\n"
"No existing outputs will be modified unless \"subtractFeeFromOutputs\" is specified.\n"
"Note that inputs which were signed may need to be resigned after completion since in/outputs have been added.\n"
"The inputs added will not be signed, use signrawtransaction for that.\n"
"Note that all existing inputs must have their previous output transaction be in the wallet.\n"
@ -2481,6 +2482,12 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
" \"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 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"
" The outputs are specified by their zero-based index, before any change output is added.\n"
" Those recipients will receive less bitcoins than you enter in their corresponding amount field.\n"
" If no outputs are specified here, the sender pays the fee.\n"
" [vout_index,...]\n"
" }\n"
" for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}\n"
"\nResult:\n"
@ -2509,6 +2516,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
bool lockUnspents = false;
CFeeRate feeRate = CFeeRate(0);
bool overrideEstimatedFeerate = false;
UniValue subtractFeeFromOutputs;
set<int> setSubtractFeeFromOutputs;
if (request.params.size() > 1) {
if (request.params[1].type() == UniValue::VBOOL) {
@ -2527,6 +2536,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
{"includeWatching", UniValueType(UniValue::VBOOL)},
{"lockUnspents", UniValueType(UniValue::VBOOL)},
{"feeRate", UniValueType()}, // will be checked below
{"subtractFeeFromOutputs", UniValueType(UniValue::VARR)},
},
true, true);
@ -2553,6 +2563,9 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
feeRate = CFeeRate(AmountFromValue(options["feeRate"]));
overrideEstimatedFeerate = true;
}
if (options.exists("subtractFeeFromOutputs"))
subtractFeeFromOutputs = options["subtractFeeFromOutputs"].get_array();
}
}
@ -2567,10 +2580,21 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
if (changePosition != -1 && (changePosition < 0 || (unsigned int)changePosition > tx.vout.size()))
throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds");
for (unsigned int idx = 0; idx < subtractFeeFromOutputs.size(); idx++) {
int pos = subtractFeeFromOutputs[idx].get_int();
if (setSubtractFeeFromOutputs.count(pos))
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos));
if (pos < 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos));
if (pos >= int(tx.vout.size()))
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos));
setSubtractFeeFromOutputs.insert(pos);
}
CAmount nFeeOut;
string strFailReason;
if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, changeAddress))
if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, setSubtractFeeFromOutputs, changeAddress))
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
UniValue result(UniValue::VOBJ);

View file

@ -2181,14 +2181,15 @@ 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 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, const CTxDestination& destChange)
{
vector<CRecipient> vecSend;
// Turn the txout set into a CRecipient vector
BOOST_FOREACH(const CTxOut& txOut, tx.vout)
for (size_t idx = 0; idx < tx.vout.size(); idx++)
{
CRecipient recipient = {txOut.scriptPubKey, txOut.nValue, false};
const CTxOut& txOut = tx.vout[idx];
CRecipient recipient = {txOut.scriptPubKey, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1};
vecSend.push_back(recipient);
}
@ -2210,6 +2211,10 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
if (nChangePosInOut != -1)
tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]);
// Copy output sizes from new transaction; they may have had the fee subtracted from them
for (unsigned int idx = 0; idx < tx.vout.size(); idx++)
tx.vout[idx].nValue = wtx.tx->vout[idx].nValue;
// Add new txins (keeping original txin scriptSig/order)
BOOST_FOREACH(const CTxIn& txin, wtx.tx->vin)
{

View file

@ -772,7 +772,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 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, const CTxDestination& destChange = CNoDestination());
/**
* Create a new transaction paying the recipients with a set of coins