diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index 1fecbf8b165af..50a8e6727d94b 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -274,7 +274,8 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult) CTransactionRef tx; { LOCK2(pwallet->cs_wallet, cs_main); - if (!pwallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl)) { + FeeCalculation fee_calc_out; + if (!pwallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl, fee_calc_out)) { return false; } } diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 510ce0443e20f..3332c4be656e7 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -44,6 +44,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendtoaddress", 6, "use_cj" }, { "sendtoaddress", 7, "conf_target" }, { "sendtoaddress", 9, "avoid_reuse" }, + { "sendtoaddress", 10, "verbose"}, { "settxfee", 0, "amount" }, { "sethdseed", 0, "newkeypool" }, { "getreceivedbyaddress", 1, "minconf" }, @@ -89,6 +90,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmany", 6, "use_is" }, { "sendmany", 7, "use_cj" }, { "sendmany", 8, "conf_target" }, + { "sendmany", 10, "verbose" }, { "deriveaddresses", 1, "range" }, { "scantxoutset", 1, "scanobjects" }, { "addmultisigaddress", 0, "nrequired" }, diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index ce8b4e844e86e..d5fa516875640 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -270,7 +270,8 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci int nChangePos = -1; bilingual_str strFailReason; - if (!pwallet->CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, false, tx.vExtraPayload.size())) { + FeeCalculation fee_calc_out; + if (!pwallet->CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason.original); } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 3df562d1ad9ff..c1e11ab09fa23 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -271,8 +271,9 @@ class WalletImpl : public Wallet LOCK(m_wallet->cs_wallet); ReserveDestination m_dest(m_wallet.get()); CTransactionRef tx; + FeeCalculation fee_calc_out; if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos, - fail_reason, coin_control, sign)) { + fail_reason, coin_control, fee_calc_out, sign)) { return {}; } return tx; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 476b799ca3f69..33cdf376dafc9 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -382,7 +382,7 @@ void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_f } } -UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector &recipients, mapValue_t map_value) +UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector &recipients, mapValue_t map_value, bool verbose) { EnsureWalletIsUnlocked(pwallet); @@ -395,11 +395,18 @@ UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std int nChangePosRet = -1; bilingual_str error; CTransactionRef tx; - bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + FeeCalculation fee_calc_out; + bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } pwallet->CommitTransaction(tx, std::move(map_value), {} /* orderForm */); + if (verbose) { + UniValue entry(UniValue::VOBJ); + entry.pushKV("txid", tx->GetHash().GetHex()); + entry.pushKV("fee_reason", StringForFeeReason(fee_calc_out.reason)); + return entry; + } return tx->GetHash().GetHex(); } @@ -425,9 +432,19 @@ static RPCHelpMan sendtoaddress() " \"" + FeeModes("\"\n\"") + "\""}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" " dirty if they have previously been used in a transaction."}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra information about the transaction."}, }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id." + { + RPCResult{"if verbose is not set or set to false", + RPCResult::Type::STR_HEX, "txid", "The transaction id." + }, + RPCResult{"if verbose is set to true", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, + {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + }, + }, }, RPCExamples{ HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") @@ -486,8 +503,9 @@ static RPCHelpMan sendtoaddress() std::vector recipients; ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + bool verbose = request.params[10].isNull() ? false: request.params[10].get_bool(); - return SendMoney(pwallet, coin_control, recipients, mapValue); + return SendMoney(pwallet, coin_control, recipients, mapValue, verbose); }, }; } @@ -917,11 +935,22 @@ static RPCHelpMan sendmany() {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."}, + }, + { + RPCResult{"if verbose is not set or set to false", + RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" + "the number of addresses." + }, + RPCResult{"if verbose is set to true", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" + "the number of addresses."}, + {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + }, + }, }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" - "the number of addresses." - }, RPCExamples{ "\nSend two amounts to two different addresses:\n" + HelpExampleCli("sendmany", "\"\" \"{\\\"" + EXAMPLE_ADDRESS[0] + "\\\":0.01,\\\"" + EXAMPLE_ADDRESS[1] + "\\\":0.02}\"") + @@ -966,8 +995,9 @@ static RPCHelpMan sendmany() std::vector recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + bool verbose = request.params[10].isNull() ? false : request.params[10].get_bool(); - return SendMoney(pwallet, coin_control, recipients, std::move(mapValue)); + return SendMoney(pwallet, coin_control, recipients, std::move(mapValue), verbose); }, }; } @@ -4651,8 +4681,8 @@ static const CRPCCommand commands[] = { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","options"} }, - { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse"} }, + { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode","verbose"} }, + { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse","verbose"} }, { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, { "wallet", "setcoinjoinrounds", &setcoinjoinrounds, {"rounds"} }, { "wallet", "setcoinjoinamount", &setcoinjoinamount, {"amount"} }, diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index e53dc78eb8974..94d3f4d54cb71 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -185,7 +185,8 @@ class CTransactionBuilderTestSetup : public TestChain100Setup } for (CAmount nAmount : vecAmounts) { CTransactionRef tx; - BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl, fee_calc_out)); { LOCK2(wallet->cs_wallet, cs_main); wallet->CommitTransaction(tx, {}, {}); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 0115d90503c14..d27d72b2eec7c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -602,7 +602,8 @@ class ListCoinsTestingSetup : public TestChain100Setup int changePos = -1; bilingual_str error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; { @@ -756,7 +757,8 @@ class CreateTransactionTestSetup : public TestChain100Setup int nChangePos = nChangePosRequest; bilingual_str strError; - bool fCreationSucceeded = wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePos, strError, coinControl); + FeeCalculation fee_calc_out; + bool fCreationSucceeded = wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePos, strError, coinControl, fee_calc_out); bool fHitMaxTries = strError.original == strExceededMaxTries; // This should never happen. if (fHitMaxTries) { @@ -808,7 +810,8 @@ class CreateTransactionTestSetup : public TestChain100Setup int nChangePosRet = -1; bilingual_str strError; CCoinControl coinControl; - BOOST_CHECK(wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePosRet, strError, coinControl)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePosRet, strError, coinControl, fee_calc_out)); wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; { @@ -1147,10 +1150,11 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup int changePos = -1; bilingual_str error; CCoinControl dummy; + FeeCalculation fee_calc_out; BOOST_CHECK(wallet->CreateTransaction({CRecipient{GetScriptForRawPubKey({}), 2 * COIN, true /* subtract fee */}}, - tx1, fee, changePos, error, dummy)); + tx1, fee, changePos, error, dummy, fee_calc_out)); BOOST_CHECK(wallet->CreateTransaction({CRecipient{GetScriptForRawPubKey({}), 1 * COIN, true /* subtract fee */}}, - tx2, fee, changePos, error, dummy)); + tx2, fee, changePos, error, dummy, fee_calc_out)); wallet->CommitTransaction(tx1, {}, {}); BOOST_CHECK_EQUAL(wallet->GetAvailableBalance(), 0); CreateAndProcessBlock({CMutableTransaction(*tx2)}, GetScriptForRawPubKey({})); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 545a0ddf829d9..b727fb5212353 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3080,7 +3080,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC LOCK(cs_wallet); CTransactionRef tx_new; - if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, false, tx.vExtraPayload.size())) { + FeeCalculation fee_calc_out; + if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { return false; } @@ -3387,7 +3388,8 @@ bool CWallet::GetBudgetSystemCollateralTX(CTransactionRef& tx, uint256 hash, CAm if (!outpoint.IsNull()) { coinControl.Select(outpoint); } - bool success = CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, error, coinControl); + FeeCalculation fee_calc_out; + bool success = CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, error, coinControl, fee_calc_out); if(!success){ WalletLogPrintf("CWallet::GetBudgetSystemCollateralTX -- Error: %s\n", error.original); return false; @@ -3403,6 +3405,7 @@ bool CWallet::CreateTransactionInternal( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) { @@ -3761,6 +3764,7 @@ bool CWallet::CreateTransactionInternal( // Before we return success, we assume any change key will be used to prevent // accidental re-use. reservedest.KeepDestination(); + fee_calc_out = feeCalc; WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, @@ -3780,12 +3784,13 @@ bool CWallet::CreateTransaction( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) { int nChangePosIn = nChangePosInOut; Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr) - bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, sign, nExtraPayloadSize); + bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign, nExtraPayloadSize); // try with avoidpartialspends unless it's enabled already if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { CCoinControl tmp_cc = coin_control; @@ -3801,7 +3806,7 @@ bool CWallet::CreateTransaction( CTransactionRef tx2; int nChangePosInOut2 = nChangePosIn; bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results - if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, sign, nExtraPayloadSize)) { + if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, fee_calc_out, sign, nExtraPayloadSize)) { // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps = nFeeRet2 <= nFeeRet + m_max_aps_fee; WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped"); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7cb32f4c0a736..ec45be2c44ada 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -842,7 +842,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure std::map> m_spk_managers; - bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign, int nExtraPayloadSize); + bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize); public: /** @@ -1133,7 +1133,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * selected by SelectCoins(); Also create the change output, when needed * @note passing nChangePosInOut as -1 will result in setting a random position */ - bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign = true, int nExtraPayloadSize = 0); + bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true, int nExtraPayloadSize = 0); /** * Submit the transaction to the node's mempool and then relay to peers. * Should be called after CreateTransaction unless you want to abort diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 9020a72329429..3b3214764d053 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -673,6 +673,17 @@ def run_test(self): assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"])) + self.log.info("Test send* RPCs with verbose=True") + address = self.nodes[0].getnewaddress("test") + txid_feeReason_one = self.nodes[2].sendtoaddress(address = address, amount = 5, verbose = True) + assert_equal(txid_feeReason_one["fee_reason"], "Fallback fee") + txid_feeReason_two = self.nodes[2].sendmany(dummy = '', amounts = {address: 5}, verbose = True) + assert_equal(txid_feeReason_two["fee_reason"], "Fallback fee") + self.log.info("Test send* RPCs with verbose=False") + txid_feeReason_three = self.nodes[2].sendtoaddress(address = address, amount = 5, verbose = False) + assert_equal(self.nodes[2].gettransaction(txid_feeReason_three)['txid'], txid_feeReason_three) + txid_feeReason_four = self.nodes[2].sendmany(dummy = '', amounts = {address: 5}, verbose = False) + assert_equal(self.nodes[2].gettransaction(txid_feeReason_four)['txid'], txid_feeReason_four) if __name__ == '__main__': WalletTest().main()