Retropex/bitcoin
1
0
Fork 0
mirror of https://github.com/Retropex/bitcoin.git synced 2025-05-28 21:12:29 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

wallet: CreateTransaction(): return out-params as (optional) struct

Browse Source
This commit is contained in:
Sebastian Falbesoner 2020-12-13 03:15:40 +01:00
parent c9fdaa5e3a
commit 4c5ceb040c
7 changed files with 52 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/wallet/feebumper.cpp
View File

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

13
src/wallet/interfaces.cpp
View File

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

9
src/wallet/rpc/spend.cpp
View File

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

62
src/wallet/spend.cpp
View File

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

6
src/wallet/spend.h
View File

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

15
src/wallet/test/spend_tests.cpp
View File

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

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

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