aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/wallet/feebumper.cpp11
-rw-r--r--src/wallet/interfaces.cpp13
-rw-r--r--src/wallet/rpc/spend.cpp9
-rw-r--r--src/wallet/spend.cpp62
-rw-r--r--src/wallet/spend.h6
-rw-r--r--src/wallet/test/spend_tests.cpp15
-rw-r--r--src/wallet/test/wallet_tests.cpp7
7 files changed, 52 insertions, 71 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 73042424ad..afd2b83971 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -218,21 +218,20 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
// We cannot source new unconfirmed inputs(bip125 rule 2)
new_coin_control.m_min_depth = 1;
- CTransactionRef tx_new;
- CAmount fee_ret;
- int change_pos_in_out = -1; // No requested location for change
+ constexpr int RANDOM_CHANGE_POSITION = -1;
bilingual_str fail_reason;
FeeCalculation fee_calc_out;
- if (!CreateTransaction(wallet, recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, fee_calc_out, false)) {
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, fail_reason, new_coin_control, fee_calc_out, false);
+ if (!txr) {
errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason);
return Result::WALLET_ERROR;
}
// Write back new fee if successful
- new_fee = fee_ret;
+ new_fee = txr->fee;
// Write back transaction
- mtx = CMutableTransaction(*tx_new);
+ mtx = CMutableTransaction(*txr->tx);
return Result::OK;
}
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 98e843385c..b269137254 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -257,13 +257,14 @@ public:
bilingual_str& fail_reason) override
{
LOCK(m_wallet->cs_wallet);
- CTransactionRef tx;
FeeCalculation fee_calc_out;
- if (!CreateTransaction(*m_wallet, recipients, tx, fee, change_pos,
- fail_reason, coin_control, fee_calc_out, sign)) {
- return {};
- }
- return tx;
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(*m_wallet, recipients, change_pos,
+ fail_reason, coin_control, fee_calc_out, sign);
+ if (!txr) return {};
+ fee = txr->fee;
+ change_pos = txr->change_pos;
+
+ return txr->tx;
}
void commitTransaction(CTransactionRef tx,
WalletValueMap value_map,
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 07119133b7..3d975b5402 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -155,15 +155,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
std::shuffle(recipients.begin(), recipients.end(), FastRandomContext());
// Send
- CAmount nFeeRequired = 0;
- int nChangePosRet = -1;
+ constexpr int RANDOM_CHANGE_POSITION = -1;
bilingual_str error;
- CTransactionRef tx;
FeeCalculation fee_calc_out;
- const bool fCreated = CreateTransaction(wallet, recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true);
- if (!fCreated) {
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true);
+ if (!txr) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
}
+ CTransactionRef tx = txr->tx;
wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */);
if (verbose) {
UniValue entry(UniValue::VOBJ);
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 7bef67d235..e5fd7b0eb4 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -961,14 +961,10 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut);
}
-// TODO: also return std::optional<CreatedTransactionResult> here in order
-// to eliminate the out parameters and to simplify the function
-bool CreateTransaction(
+std::optional<CreatedTransactionResult> CreateTransaction(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
- CTransactionRef& tx,
- CAmount& nFeeRet,
- int& nChangePosInOut,
+ int change_pos,
bilingual_str& error,
const CCoinControl& coin_control,
FeeCalculation& fee_calc_out,
@@ -976,54 +972,37 @@ bool CreateTransaction(
{
if (vecSend.empty()) {
error = _("Transaction must have at least one recipient");
- return false;
+ return std::nullopt;
}
if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) {
error = _("Transaction amounts must not be negative");
- return false;
+ return std::nullopt;
}
LOCK(wallet.cs_wallet);
- int nChangePosIn = nChangePosInOut;
- Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr)
- std::optional<CreatedTransactionResult> txr_ungrouped = CreateTransactionInternal(wallet, vecSend, nChangePosInOut, error, coin_control, fee_calc_out, sign);
- bool res = false;
- if (txr_ungrouped) {
- tx = txr_ungrouped->tx;
- nFeeRet = txr_ungrouped->fee;
- nChangePosInOut = txr_ungrouped->change_pos;
- res = true;
- }
- TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res, nFeeRet, nChangePosInOut);
+ std::optional<CreatedTransactionResult> txr_ungrouped = CreateTransactionInternal(wallet, vecSend, change_pos, error, coin_control, fee_calc_out, sign);
+ TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), txr_ungrouped.has_value(),
+ txr_ungrouped.has_value() ? txr_ungrouped->fee : 0, txr_ungrouped.has_value() ? txr_ungrouped->change_pos : 0);
+ if (!txr_ungrouped) return std::nullopt;
// try with avoidpartialspends unless it's enabled already
- if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
+ if (txr_ungrouped->fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str());
CCoinControl tmp_cc = coin_control;
tmp_cc.m_avoid_partial_spends = true;
- CAmount nFeeRet2;
- CTransactionRef tx2;
- int nChangePosInOut2 = nChangePosIn;
bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results
- std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, nChangePosInOut2, error2, tmp_cc, fee_calc_out, sign);
+ std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign);
if (txr_grouped) {
- tx2 = txr_grouped->tx;
- nFeeRet2 = txr_grouped->fee;
- nChangePosInOut2 = txr_grouped->change_pos;
-
// if fee of this alternative one is within the range of the max fee, we use this one
- const bool use_aps = nFeeRet2 <= nFeeRet + wallet.m_max_aps_fee;
- wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped");
- TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, res, nFeeRet2, nChangePosInOut2);
- if (use_aps) {
- tx = tx2;
- nFeeRet = nFeeRet2;
- nChangePosInOut = nChangePosInOut2;
- }
+ const bool use_aps = txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee;
+ wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n",
+ txr_ungrouped->fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped");
+ TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, true, txr_grouped->fee, txr_grouped->change_pos);
+ if (use_aps) return txr_grouped;
}
}
- return res;
+ return txr_ungrouped;
}
bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl)
@@ -1047,11 +1026,12 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
// CreateTransaction call and LockCoin calls (when lockUnspents is true).
LOCK(wallet.cs_wallet);
- CTransactionRef tx_new;
FeeCalculation fee_calc_out;
- if (!CreateTransaction(wallet, vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, fee_calc_out, false)) {
- return false;
- }
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, vecSend, nChangePosInOut, error, coinControl, fee_calc_out, false);
+ if (!txr) return false;
+ CTransactionRef tx_new = txr->tx;
+ nFeeRet = txr->fee;
+ nChangePosInOut = txr->change_pos;
if (nChangePosInOut != -1) {
tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index ca5714245e..8af712110d 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -10,6 +10,8 @@
#include <wallet/transaction.h>
#include <wallet/wallet.h>
+#include <optional>
+
namespace wallet {
/** Get the marginal bytes if spending the specified output from this transaction.
* use_max_sig indicates whether to use the maximum sized, 72 byte signature when calculating the
@@ -95,9 +97,9 @@ struct CreatedTransactionResult
/**
* Create a new transaction paying the recipients with a set of coins
* selected by SelectCoins(); Also create the change output, when needed
- * @note passing nChangePosInOut as -1 will result in setting a random position
+ * @note passing change_pos as -1 will result in setting a random position
*/
-bool CreateTransaction(CWallet& wallet, 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);
+std::optional<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true);
/**
* Insert additional inputs into the transaction by
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index 334bd5b8bc..bdc148afb4 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -27,9 +27,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
// instead of the miner.
auto check_tx = [&wallet](CAmount leftover_input_amount) {
CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, true /* subtract fee */};
- CTransactionRef tx;
- CAmount fee;
- int change_pos = -1;
+ constexpr int RANDOM_CHANGE_POSITION = -1;
bilingual_str error;
CCoinControl coin_control;
coin_control.m_feerate.emplace(10000);
@@ -37,11 +35,12 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
// We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output
coin_control.m_change_type = OutputType::LEGACY;
FeeCalculation fee_calc;
- BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, change_pos, error, coin_control, fee_calc));
- BOOST_CHECK_EQUAL(tx->vout.size(), 1);
- BOOST_CHECK_EQUAL(tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - fee);
- BOOST_CHECK_GT(fee, 0);
- return fee;
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc);
+ BOOST_CHECK(txr.has_value());
+ BOOST_CHECK_EQUAL(txr->tx->vout.size(), 1);
+ BOOST_CHECK_EQUAL(txr->tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr->fee);
+ BOOST_CHECK_GT(txr->fee, 0);
+ return txr->fee;
};
// Send full input amount to recipient, check that only nonzero fee is
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 683f0eb327..d4406fd5dd 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -521,13 +521,14 @@ public:
CWalletTx& AddTx(CRecipient recipient)
{
CTransactionRef tx;
- CAmount fee;
- int changePos = -1;
bilingual_str error;
CCoinControl dummy;
FeeCalculation fee_calc_out;
{
- BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, changePos, error, dummy, fee_calc_out));
+ constexpr int RANDOM_CHANGE_POSITION = -1;
+ std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, dummy, fee_calc_out);
+ BOOST_CHECK(txr.has_value());
+ tx = txr->tx;
}
wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx;
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-15 09:37:17 +0000