From d5863c0b3e20d56acf7246008b7832efde68ab21 Mon Sep 17 00:00:00 2001 From: Sishir Giri Date: Sun, 12 Jul 2020 12:52:02 -0700 Subject: [send] Make send RPCs return fee reason --- src/interfaces/wallet.cpp | 3 ++- src/rpc/client.cpp | 2 ++ src/wallet/feebumper.cpp | 3 ++- src/wallet/rpcwallet.cpp | 55 +++++++++++++++++++++++++++++++--------- src/wallet/test/wallet_tests.cpp | 3 ++- src/wallet/wallet.cpp | 10 +++++--- src/wallet/wallet.h | 4 +-- test/functional/wallet_basic.py | 1 - 8 files changed, 60 insertions(+), 21 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 94c63da84e..f68016b557 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -231,8 +231,9 @@ public: { LOCK(m_wallet->cs_wallet); 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/rpc/client.cpp b/src/rpc/client.cpp index 3c432464f2..88c8ebe1f6 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -41,6 +41,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendtoaddress", 5 , "replaceable" }, { "sendtoaddress", 6 , "conf_target" }, { "sendtoaddress", 8, "avoid_reuse" }, + { "sendtoaddress", 9, "verbose"}, { "settxfee", 0, "amount" }, { "sethdseed", 0, "newkeypool" }, { "getreceivedbyaddress", 1, "minconf" }, @@ -72,6 +73,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmany", 4, "subtractfeefrom" }, { "sendmany", 5 , "replaceable" }, { "sendmany", 6 , "conf_target" }, + { "sendmany", 8, "verbose" }, { "deriveaddresses", 1, "range" }, { "scantxoutset", 1, "scanobjects" }, { "addmultisigaddress", 0, "nrequired" }, diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index cacf306891..6cbad14de8 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -219,7 +219,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo CAmount fee_ret; int change_pos_in_out = -1; // No requested location for change bilingual_str fail_reason; - if (!wallet.CreateTransaction(recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) { + FeeCalculation fee_calc_out; + if (!wallet.CreateTransaction(recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, fee_calc_out, false)) { errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason); return Result::WALLET_ERROR; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2217bc2875..45616431fb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -397,7 +397,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); @@ -409,11 +409,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(); } @@ -438,9 +445,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") @@ -497,8 +514,9 @@ static RPCHelpMan sendtoaddress() std::vector recipients; ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + bool verbose = request.params[9].isNull() ? false: request.params[9].get_bool(); - return SendMoney(pwallet, coin_control, recipients, mapValue); + return SendMoney(pwallet, coin_control, recipients, mapValue, verbose); }, }; } @@ -853,11 +871,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}\"") + @@ -902,12 +931,14 @@ static RPCHelpMan sendmany() std::vector recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + bool verbose = request.params[8].isNull() ? false : request.params[8].get_bool(); - return SendMoney(pwallet, coin_control, recipients, std::move(mapValue)); + return SendMoney(pwallet, coin_control, recipients, std::move(mapValue), verbose); }, }; } + static RPCHelpMan addmultisigaddress() { return RPCHelpMan{"addmultisigaddress", @@ -4501,8 +4532,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","comment","subtractfeefrom","replaceable","conf_target","estimate_mode"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse"} }, + { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode","verbose"} }, + { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse","verbose"} }, { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, { "wallet", "setlabel", &setlabel, {"address","label"} }, { "wallet", "settxfee", &settxfee, {"amount"} }, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 4393bb7701..c42114c394 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -524,8 +524,9 @@ public: int changePos = -1; bilingual_str error; CCoinControl dummy; + FeeCalculation fee_calc_out; { - BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy)); + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 66857dbb39..d97ec019a8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2631,7 +2631,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)) { + FeeCalculation fee_calc_out; + if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, fee_calc_out, false)) { return false; } @@ -2755,6 +2756,7 @@ bool CWallet::CreateTransactionInternal( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign) { CAmount nValue = 0; @@ -3098,6 +3100,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 Needed:%d 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, nFeeNeeded, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, @@ -3117,11 +3120,12 @@ bool CWallet::CreateTransaction( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign) { int nChangePosIn = nChangePosInOut; CTransactionRef tx2 = tx; - bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, sign); + bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign); // 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; @@ -3129,7 +3133,7 @@ bool CWallet::CreateTransaction( CAmount nFeeRet2; 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)) { + if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, fee_calc_out, sign)) { // 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 f15712dd0e..3a1f087603 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -723,7 +723,7 @@ private: // 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); + 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); public: /* @@ -974,7 +974,7 @@ public: * 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); + 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); /** * 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 bb208341a0..4f1cadc835 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -661,6 +661,5 @@ class WalletTest(BitcoinTestFramework): assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"])) - if __name__ == '__main__': WalletTest().main() -- cgit v1.2.3