Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/interfaces/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,9 @@ class WalletImpl : public Wallet
{
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;
Expand Down
2 changes: 2 additions & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down Expand Up @@ -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" },
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/feebumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
55 changes: 43 additions & 12 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CRecipient> &recipients, mapValue_t map_value)
UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector<CRecipient> &recipients, mapValue_t map_value, bool verbose)
{
EnsureWalletIsUnlocked(pwallet);

Expand All @@ -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();
}

Expand All @@ -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")
Expand Down Expand Up @@ -497,8 +514,9 @@ static RPCHelpMan sendtoaddress()

std::vector<CRecipient> recipients;
ParseRecipients(address_amounts, subtractFeeFromAmount, recipients);
bool verbose = request.params[9].isNull() ? false: request.params[9].get_bool();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

micro-nit: spacing between false:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason git is ignoring my whitespace changes. Any suggestions on how to fix that?


return SendMoney(pwallet, coin_control, recipients, mapValue);
return SendMoney(pwallet, coin_control, recipients, mapValue, verbose);
},
};
}
Expand Down Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

micro-nit: double space between Only 1

"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}\"") +
Expand Down Expand Up @@ -902,12 +931,14 @@ static RPCHelpMan sendmany()

std::vector<CRecipient> 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",
Expand Down Expand Up @@ -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"} },
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/test/wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,9 @@ class ListCoinsTestingSetup : public TestChain100Setup
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;
Expand Down
10 changes: 7 additions & 3 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -3117,19 +3120,20 @@ 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;
tmp_cc.m_avoid_partial_spends = true;
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");
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,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<uint256, std::unique_ptr<ScriptPubKeyMan>> m_spk_managers;

bool CreateTransactionInternal(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign);
bool CreateTransactionInternal(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign);

public:
/*
Expand Down Expand Up @@ -974,7 +974,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<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign = true);
bool CreateTransaction(const std::vector<CRecipient>& 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
Expand Down
11 changes: 11 additions & 0 deletions test/functional/wallet_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a case where you explicitly pass in verbose = False to cover that case too 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added verbose = False case in test file

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()