From 523b24d43d63e350b0792ed96b4c2c1983cb147b Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 26 Apr 2021 18:36:15 +0200 Subject: [PATCH] Subtract fee from amount >>> adapts bitcoin@292623adf59edea52b2927b3d67fd2ff7f997882 + bitcoin@61718268b5067acd1b8af4a4e94b1bf60334e1f7 Adds the autom-subtract-the-fee-from-the-amount-and-send-whats-left feature to the RPC (sendtoaddress and fundrawtransaction only for now). --- src/rpc/client.cpp | 1 + src/rpc/rawtransaction.cpp | 31 ++++++++++++++- src/rpc/rpcevo.cpp | 2 +- src/wallet/rpcwallet.cpp | 22 +++++++---- src/wallet/wallet.cpp | 45 ++++++++++++++++------ src/wallet/wallet.h | 2 +- test/functional/wallet_basic.py | 11 +++--- test/functional/wallet_listtransactions.py | 21 ++++++++++ 8 files changed, 107 insertions(+), 28 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 002d4b91e6a0f..2336d06430f63 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -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" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 4669b0334a521..c4c38d3c77ee8 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -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" @@ -542,6 +548,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) int changePosition = -1; bool includeWatching = false; bool lockUnspents = false; + UniValue subtractFeeFromOutputs; + std::set setSubtractFeeFromOutputs; CFeeRate feeRate = CFeeRate(0); bool overrideEstimatedFeerate = false; @@ -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); @@ -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 @@ -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)); diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index fb5d621ed5732..bb2e46223f99f 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -224,7 +224,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, SpecialTxPa int nChangePos = -1; std::string strFailReason; std::set 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) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f9eeeb32496de..0206742137643 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -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); @@ -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 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); @@ -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" @@ -1100,6 +1104,8 @@ 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" @@ -1107,6 +1113,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request) "\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); @@ -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); @@ -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()); @@ -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"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3e084d7eda065..3cbb4d3038e61 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -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& setSubtractFeeFromOutputs, const CTxDestination& destChange) { std::vector 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; @@ -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)) { @@ -3003,16 +3010,18 @@ bool CWallet::CreateTransaction(const std::vector& 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; } @@ -3036,11 +3045,25 @@ bool CWallet::CreateTransaction(const std::vector& 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; @@ -3052,13 +3075,13 @@ bool CWallet::CreateTransaction(const std::vector& 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 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3b354245dfd8d..8f8185519e435 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -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& 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 diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 3d5b04d62730e..ddea9344ed8a6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -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) @@ -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) @@ -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() @@ -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 @@ -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 @@ -258,7 +259,5 @@ def run_test(self): assert_equal(9, self.len_listunspent({"minimumSumAmount": 2500.00})) - - if __name__ == '__main__': WalletTest().main() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 20222cc4b968b..3f8bccc6ab642 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -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, ) @@ -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()