Skip to content

Commit

Permalink
Subtract fee from amount
Browse files Browse the repository at this point in the history
>>> adapts bitcoin/bitcoin@292623a
         + bitcoin/bitcoin@6171826

Adds the autom-subtract-the-fee-from-the-amount-and-send-whats-left
feature to the RPC (sendtoaddress and fundrawtransaction only for now).
  • Loading branch information
random-zebra committed Jun 9, 2021
1 parent 2d50b6e commit 523b24d
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ static const CRPCConvertParam vRPCConvertParams[] = {
{ "sendmany", 2, "minconf" },
{ "sendrawtransaction", 1, "allowhighfees" },
{ "sendtoaddress", 1, "amount" },
{ "sendtoaddress", 4, "subtract_fee" },
{ "setautocombinethreshold", 0, "enable" },
{ "setautocombinethreshold", 1, "threshold" },
{ "setban", 2, "bantime" },
Expand Down
31 changes: 29 additions & 2 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,13 @@ 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"
" \"feeRate\" (numeric, optional, default 0=estimate) Set a specific feerate (PIV per KB)\n"
" \"feeRate\" (numeric, optional, default 0=estimate) Set a specific feerate (PIV 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 PIV 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"
"\nResult:\n"
"{\n"
Expand Down Expand Up @@ -542,6 +548,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
int changePosition = -1;
bool includeWatching = false;
bool lockUnspents = false;
UniValue subtractFeeFromOutputs;
std::set<int> setSubtractFeeFromOutputs;
CFeeRate feeRate = CFeeRate(0);
bool overrideEstimatedFeerate = false;

Expand All @@ -555,6 +563,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);

Expand All @@ -578,6 +587,10 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
feeRate = CFeeRate(AmountFromValue(options["feeRate"]));
overrideEstimatedFeerate = true;
}

if (options.exists("subtractFeeFromOutputs")) {
subtractFeeFromOutputs = options["subtractFeeFromOutputs"].get_array();
}
}

// parse hex string from parameter
Expand All @@ -591,11 +604,25 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
if (changePosition != -1 && (changePosition < 0 || (unsigned int) changePosition > origTx.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(origTx.vout.size()))
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos));
setSubtractFeeFromOutputs.insert(pos);
}

CMutableTransaction tx(origTx);
CAmount nFeeOut;
std::string strFailReason;
if(!pwallet->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, changeAddress))
if(!pwallet->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate,
changePosition, strFailReason, includeWatching,
lockUnspents, setSubtractFeeFromOutputs, changeAddress)) {
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);
}

UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(tx));
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rpcevo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, SpecialTxPa
int nChangePos = -1;
std::string strFailReason;
std::set<int> setSubtractFeeFromOutputs;
if (!pwallet->FundTransaction(tx, nFee, false, feeRate, nChangePos, strFailReason, false, false))
if (!pwallet->FundTransaction(tx, nFee, false, feeRate, nChangePos, strFailReason, false, false, {}))
throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason);

if (dummyTxOutAdded && tx.vout.size() > 1) {
Expand Down
22 changes: 15 additions & 7 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ UniValue setlabel(const JSONRPCRequest& request)
return NullUniValue;
}

static void SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, CTransactionRef& tx)
static void SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, bool fSubtractFeeFromAmount, CTransactionRef& tx)
{
LOCK2(cs_main, pwallet->cs_wallet);

Expand All @@ -1018,8 +1018,12 @@ static void SendMoney(CWallet* const pwallet, const CTxDestination& address, CAm
CReserveKey reservekey(pwallet);
CAmount nFeeRequired;
std::string strError;
if (!pwallet->CreateTransaction(scriptPubKey, nValue, tx, reservekey, nFeeRequired, strError, nullptr, (CAmount)0)) {
if (nValue + nFeeRequired > pwallet->GetAvailableBalance())
std::vector<CRecipient> vecSend;
int nChangePosRet = -1;
CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
if (!pwallet->CreateTransaction(vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError)) {
if (!fSubtractFeeFromAmount && nValue + nFeeRequired > pwallet->GetAvailableBalance())
strError = strprintf("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds!", FormatMoney(nFeeRequired));
LogPrintf("%s: %s\n", __func__, strError);
throw JSONRPCError(RPC_WALLET_ERROR, strError);
Expand Down Expand Up @@ -1086,9 +1090,9 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
if (!EnsureWalletIsAvailable(pwallet, request.fHelp))
return NullUniValue;

if (request.fHelp || request.params.size() < 2 || request.params.size() > 4)
if (request.fHelp || request.params.size() < 2 || request.params.size() > 5)
throw std::runtime_error(
"sendtoaddress \"address\" amount ( \"comment\" \"comment-to\" )\n"
"sendtoaddress \"address\" amount ( \"comment\" \"comment-to\" subtract_fee )\n"
"\nSend an amount to a given address. The amount is a real and is rounded to the nearest 0.00000001\n" +
HelpRequiringPassphrase(pwallet) + "\n"

Expand All @@ -1100,13 +1104,16 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
"4. \"comment-to\" (string, optional) A comment to store the name of the person or organization \n"
" to which you're sending the transaction. This is not part of the \n"
" transaction, just kept in your wallet.\n"
"5. subtract_fee (boolean, optional, default=false) The fee will be deducted from the amount being sent.\n"
" The recipient will receive less bitcoins than you enter in the amount field.\n"

"\nResult:\n"
"\"transactionid\" (string) The transaction id.\n"

"\nExamples:\n" +
HelpExampleCli("sendtoaddress", "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\" 0.1") +
HelpExampleCli("sendtoaddress", "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\" 0.1 \"donation\" \"seans outpost\"") +
HelpExampleCli("sendtoaddress", "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\" 0.1 \"\" \"\" true") +
HelpExampleRpc("sendtoaddress", "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\", 0.1, \"donation\", \"seans outpost\""));

EnsureWalletIsUnlocked(pwallet);
Expand All @@ -1124,6 +1131,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
request.params[2].get_str() : "";
const std::string toStr = (request.params.size() > 3 && !request.params[3].isNull()) ?
request.params[3].get_str() : "";
bool fSubtractFeeFromAmount = request.params.size() > 4 && request.params[4].get_bool();

if (isShielded) {
UniValue sendTo(UniValue::VOBJ);
Expand All @@ -1137,7 +1145,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
CAmount nAmount = AmountFromValue(request.params[1]);

CTransactionRef tx;
SendMoney(pwallet, address, nAmount, tx);
SendMoney(pwallet, address, nAmount, fSubtractFeeFromAmount, tx);

// Wallet comments
CWalletTx& wtx = pwallet->mapWallet.at(tx->GetHash());
Expand Down Expand Up @@ -4556,7 +4564,7 @@ static const CRPCCommand commands[] =
{ "wallet", "lockunspent", &lockunspent, true, {"unlock","transactions"} },
{ "wallet", "rawdelegatestake", &rawdelegatestake, false, {"staking_addr","amount","owner_addr","ext_owner","include_delegated","from_shield","force"} },
{ "wallet", "sendmany", &sendmany, false, {"dummy","amounts","minconf","comment","include_delegated"} },
{ "wallet", "sendtoaddress", &sendtoaddress, false, {"address","amount","comment","comment-to"} },
{ "wallet", "sendtoaddress", &sendtoaddress, false, {"address","amount","comment","comment-to","subtract_fee"} },
{ "wallet", "settxfee", &settxfee, true, {"amount"} },
{ "wallet", "setstakesplitthreshold", &setstakesplitthreshold, false, {"value"} },
{ "wallet", "signmessage", &signmessage, true, {"address","message"} },
Expand Down
45 changes: 34 additions & 11 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2937,13 +2937,14 @@ bool CWallet::CreateBudgetFeeTX(CTransactionRef& tx, const uint256& hash, CReser
return true;
}

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)
{
std::vector<CRecipient> vecSend;

// Turn the txout set into a CRecipient vector
for (const CTxOut& txOut : tx.vout) {
vecSend.emplace_back(txOut.scriptPubKey, txOut.nValue, false);
// Turn the txout set into a CRecipient vector.
for (size_t idx = 0; idx < tx.vout.size(); idx++) {
const CTxOut& txOut = tx.vout[idx];
vecSend.emplace_back(txOut.scriptPubKey, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1);
}

CCoinControl coinControl;
Expand Down Expand Up @@ -2977,6 +2978,12 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
reservekey.KeepKey();
}

// 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->vout[idx].nValue;
}

// Add new txins while keeping original txin scriptSig/order.
for (const CTxIn& txin : wtx->vin) {
if (!coinControl.IsSelected(txin.prevout)) {
Expand All @@ -3003,16 +3010,18 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend,
{
CAmount nValue = 0;
int nChangePosRequest = nChangePosInOut;

unsigned int nSubtractFeeFromAmount = 0;
for (const CRecipient& rec : vecSend) {
if (rec.nAmount < 0) {
if (nValue < 0 || rec.nAmount < 0) {
strFailReason = _("Transaction amounts must be positive");
return false;
}
nValue += rec.nAmount;
if (rec.fSubtractFeeFromAmount)
nSubtractFeeFromAmount++;
}
if (vecSend.empty() || nValue < 0) {
strFailReason = _("Transaction amounts must be positive");
if (vecSend.empty()) {
strFailReason = _("Transaction must have at least one recipient");
return false;
}

Expand All @@ -3036,11 +3045,25 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend,
nChangePosInOut = nChangePosRequest;
txNew.vin.clear();
txNew.vout.clear();
CAmount nTotalValue = nValue + nFeeRet;
bool fFirst = true;

CAmount nValueToSelect = nValue;
if (nSubtractFeeFromAmount == 0)
nValueToSelect += nFeeRet;

// Fill outputs
for (const CRecipient& rec : vecSend) {
CTxOut txout(rec.nAmount, rec.scriptPubKey);
if (rec.fSubtractFeeFromAmount) {
assert(nSubtractFeeFromAmount != 0);
txout.nValue -= nFeeRet / nSubtractFeeFromAmount; // Subtract fee equally from each selected recipient

if (fFirst) {
// first receiver pays the remainder not divisible by output count
fFirst = false;
txout.nValue -= nFeeRet % nSubtractFeeFromAmount;
}
}
if (IsDust(txout, dustRelayFee)) {
strFailReason = _("Transaction amount too small");
return false;
Expand All @@ -3052,13 +3075,13 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend,
CAmount nValueIn = 0;
setCoins.clear();

if (!SelectCoinsToSpend(vAvailableCoins, nTotalValue, setCoins, nValueIn, coinControl)) {
if (!SelectCoinsToSpend(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl)) {
strFailReason = _("Insufficient funds.");
return false;
}

// Change
CAmount nChange = nValueIn - nValue - nFeeRet;
CAmount nChange = nValueIn - nValueToSelect;
if (nChange > 0) {
// Fill a vout to ourself
// TODO: pass in scriptChange instead of reservekey so
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
CAmount GetUnconfirmedWatchOnlyBalance() const;
CAmount GetImmatureWatchOnlyBalance() const;
CAmount GetLegacyBalance(const isminefilter& filter, int minDepth) const;
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
* selected by SelectCoins(); Also create the change output, when needed
Expand Down
11 changes: 5 additions & 6 deletions test/functional/wallet_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,11 @@ def run_test(self):
assert_equal(node_2_bal, node_2_expected_bal)

# Send 10 PIV normal
self.log.info("test sendtoaddress")
address = self.nodes[0].getnewaddress("test")
self.nodes[2].settxfee(float(fee_per_kbyte))
txid = self.nodes[2].sendtoaddress(address, 10, "", "")
fee = self.nodes[2].gettransaction(txid)["fee"]
fee = self.nodes[2].gettransaction(txid)["fee"] # fee < 0
node_2_bal -= (Decimal('10') - fee)
assert_equal(self.nodes[2].getbalance(), node_2_bal)
self.nodes[2].generate(1)
Expand All @@ -150,6 +151,7 @@ def run_test(self):
assert_equal(node_0_bal, Decimal('10'))

# Sendmany 10 PIV
self.log.info("test sendmany")
txid = self.nodes[2].sendmany('', {address: 10}, 0, "")
fee = self.nodes[2].gettransaction(txid)["fee"]
self.nodes[2].generate(1)
Expand All @@ -160,9 +162,6 @@ def run_test(self):
assert_equal(self.nodes[0].getbalance(), node_0_bal)
assert_fee_amount(-fee, self.get_vsize(self.nodes[2].getrawtransaction(txid)), fee_per_kbyte)

# This will raise an exception since generate does not accept a string
assert_raises_rpc_error(-1, "not an integer", self.nodes[0].generate, "2")

# Import address and private key to check correct behavior of spendable unspents
# 1. Send some coins to generate new UTXO
address_to_import = self.nodes[2].getnewaddress()
Expand All @@ -171,6 +170,7 @@ def run_test(self):
self.sync_all(self.nodes[0:3])

# 2. Import address from node2 to node1
self.log.info("test importaddress")
self.nodes[1].importaddress(address_to_import)

# 3. Validate that the imported address is watch-only on node1
Expand All @@ -184,6 +184,7 @@ def run_test(self):

# 5. Import private key of the previously imported address on node1
priv_key = self.nodes[2].dumpprivkey(address_to_import)
self.log.info("test importprivkey")
self.nodes[1].importprivkey(priv_key)

# 6. Check that the unspents are now spendable on node1
Expand Down Expand Up @@ -258,7 +259,5 @@ def run_test(self):
assert_equal(9, self.len_listunspent({"minimumSumAmount": 2500.00}))




if __name__ == '__main__':
WalletTest().main()
21 changes: 21 additions & 0 deletions test/functional/wallet_listtransactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from test_framework.test_framework import PivxTestFramework
from test_framework.util import (
assert_array_result,
assert_equal,
hex_str_to_bytes,
)

Expand Down Expand Up @@ -94,6 +95,26 @@ def run_test(self):
txs = [tx for tx in self.nodes[0].listtransactions("*", 100, 0, True) if "label" in tx and tx['label'] == 'watchonly']
assert_array_result(txs, {"category": "receive", "amount": Decimal("0.1")}, {"txid": txid})

node_0_bal = self.nodes[0].getbalance()
node_1_bal = self.nodes[1].getbalance()

# Send 10 PIV with subtract fee from amount
self.log.info("test sendtoaddress with subtract-fee-from-amt")
txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 10, "", "", True)
node_0_bal -= Decimal('10')
assert_equal(self.nodes[0].getbalance(), node_0_bal)
self.nodes[0].generate(1)
self.sync_all()
fee = self.nodes[0].gettransaction(txid)["fee"] # fee < 0
node_1_bal += (Decimal('10') + fee)
assert_equal(self.nodes[1].getbalance(), node_1_bal)
assert_array_result(self.nodes[0].listtransactions(),
{"txid": txid},
{"category": "send", "amount": - Decimal('10') - fee, "confirmations": 1})
assert_array_result(self.nodes[1].listtransactions(),
{"txid": txid},
{"category": "receive", "amount": + Decimal('10') + fee, "confirmations": 1})


if __name__ == '__main__':
ListTransactionsTest().main()

0 comments on commit 523b24d

Please sign in to comment.