sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 19:42:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #19501: send* RPCs in the wallet returns the "fee reason"

Browse Source 
69cf5d4eeb73f7d685e915fc17af64634d88a4a2 [test] Make sure send rpc returns fee reason (Sishir Giri)
d5863c0b3e20d56acf7246008b7832efde68ab21 [send] Make send RPCs return fee reason (Sishir Giri)

Pull request description:

  Whenever a wallet funds a transaction, the fee reason is reported to the user only if the verbose is set to true. I added an extra parameter to `CreateTransaction` function in wallet.cpp. Then I implemented the fee reason return logic in `SendMoney`  in rpcwallet.cpp, followed by verbose parameter in `sendtoaddress` and `sendmany` functions. I also added a fee reason test case in walletbasic.py.

  link to the issue: https://github.com/MarcoFalke/bitcoin-core/issues/22#issue-616251578

ACKs for top commit:
  instagibbs:
    ACK 69cf5d4eeb
  meshcollider:
    utACK 69cf5d4eeb73f7d685e915fc17af64634d88a4a2

Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
This commit is contained in:
MarcoFalke 2020-09-30 08:58:15 +02:00 committed by Konstantin Akimov
parent b6c8d852e3
commit 6a164eaea9
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
10 changed files with 83 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/coinjoin/util.cpp
View File

@ -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;
}
}

2
src/rpc/client.cpp
View File

@ -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" },

3
src/rpc/evo.cpp
View File

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

3
src/wallet/interfaces.cpp
View File

@ -271,8 +271,9 @@ public:
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;

54
src/wallet/rpcwallet.cpp
View File

@ -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<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);
@ -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<CRecipient> 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<CRecipient> 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"} },

3
src/wallet/test/coinjoin_tests.cpp
View File

@ -185,7 +185,8 @@ public:
}
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, {}, {});

14
src/wallet/test/wallet_tests.cpp
View File

@ -602,7 +602,8 @@ public:
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 @@ public:
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 @@ public:
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({}));

13
src/wallet/wallet.cpp
View File

@ -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");

4
src/wallet/wallet.h
View File

@ -842,7 +842,7 @@ private:
// 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, int nExtraPayloadSize);
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, int nExtraPayloadSize);
public:
/**
@ -1133,7 +1133,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<CRecipient>& 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<CRecipient>& 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

11
test/functional/wallet_basic.py
View File

@ -673,6 +673,17 @@ class WalletTest(BitcoinTestFramework):
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()