aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rwxr-xr-xqa/rpc-tests/fundrawtransaction.py70
-rw-r--r--qa/rpc-tests/test_framework/util.py10
-rw-r--r--src/wallet/rpcwallet.cpp38
-rw-r--r--src/wallet/wallet.cpp11
-rw-r--r--src/wallet/wallet.h2
5 files changed, 117 insertions, 14 deletions
diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py
index 82e148c55a..b97e9aecdf 100755
--- a/qa/rpc-tests/fundrawtransaction.py
+++ b/qa/rpc-tests/fundrawtransaction.py
@@ -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()
diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py
index c29033595d..aca82c8b6f 100644
--- a/qa/rpc-tests/test_framework/util.py
+++ b/qa/rpc-tests/test_framework/util.py
@@ -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)
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 82f8990562..2428ef04e2 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2479,7 +2479,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"
@@ -2491,11 +2492,17 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
"1. \"hexstring\" (string, required) The hex string of the raw transaction\n"
"2. options (object, optional)\n"
" {\n"
- " \"changeAddress\" (string, optional, default pool address) The bitcoin address to receive the change\n"
- " \"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 not set: makes wallet determine the fee) Set a specific feerate (" + CURRENCY_UNIT + " per KB)\n"
+ " \"changeAddress\" (string, optional, default pool address) The bitcoin address to receive the change\n"
+ " \"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 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"
@@ -2523,6 +2530,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) {
@@ -2541,6 +2550,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);
@@ -2567,6 +2577,9 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
feeRate = CFeeRate(AmountFromValue(options["feeRate"]));
overrideEstimatedFeerate = true;
}
+
+ if (options.exists("subtractFeeFromOutputs"))
+ subtractFeeFromOutputs = options["subtractFeeFromOutputs"].get_array();
}
}
@@ -2581,10 +2594,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);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 86816a62e2..ccda4a7c7f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2192,14 +2192,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);
}
@@ -2221,6 +2222,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)
{
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index b9fa6bb24b..92ad098486 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -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 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