aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/feebumper.cpp11
-rw-r--r--src/wallet/interfaces.cpp16
-rw-r--r--src/wallet/receive.cpp8
-rw-r--r--src/wallet/receive.h16
-rw-r--r--src/wallet/rpc/coins.cpp14
-rw-r--r--src/wallet/rpc/spend.cpp9
-rw-r--r--src/wallet/rpc/transactions.cpp1
-rw-r--r--src/wallet/spend.cpp87
-rw-r--r--src/wallet/spend.h16
-rw-r--r--src/wallet/test/spend_tests.cpp15
-rw-r--r--src/wallet/test/wallet_tests.cpp7
-rw-r--r--src/wallet/wallet.cpp23
-rw-r--r--src/wallet/wallet.h29
13 files changed, 138 insertions, 114 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..e1203817e0 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -80,7 +80,10 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
//! Construct wallet tx status struct.
WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
+ AssertLockHeld(wallet.cs_wallet);
+
WalletTxStatus result;
result.block_height =
wtx.state<TxStateConfirmed>() ? wtx.state<TxStateConfirmed>()->confirmed_block_height :
@@ -257,13 +260,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/receive.cpp b/src/wallet/receive.cpp
index cddf94aab2..8cce07b921 100644
--- a/src/wallet/receive.cpp
+++ b/src/wallet/receive.cpp
@@ -123,6 +123,8 @@ static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CW
CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
{
+ AssertLockHeld(wallet.cs_wallet);
+
// Must wait until coinbase is safely deep enough in the chain before valuing it
if (wallet.IsTxImmatureCoinBase(wtx))
return 0;
@@ -164,6 +166,8 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx)
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache)
{
+ AssertLockHeld(wallet.cs_wallet);
+
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache);
}
@@ -173,6 +177,8 @@ CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, b
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache)
{
+ AssertLockHeld(wallet.cs_wallet);
+
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache);
}
@@ -182,6 +188,8 @@ CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletT
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache, const isminefilter& filter)
{
+ AssertLockHeld(wallet.cs_wallet);
+
// Avoid caching ismine for NO or ALL cases (could remove this check and simplify in the future).
bool allow_cache = (filter & ISMINE_ALL) && (filter & ISMINE_ALL) != ISMINE_ALL;
diff --git a/src/wallet/receive.h b/src/wallet/receive.h
index d7705b5262..1caef293f2 100644
--- a/src/wallet/receive.h
+++ b/src/wallet/receive.h
@@ -24,17 +24,17 @@ bool OutputIsChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_
CAmount OutputGetChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx);
-CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
+CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
//! filter decides which addresses will count towards the debit
CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx);
-CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true);
-CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true);
-// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
-// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The
-// annotation "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid
-// having to resolve the issue of member access into incomplete type CWallet.
-CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE) NO_THREAD_SAFETY_ANALYSIS;
+CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
struct COutputEntry
{
CTxDestination destination;
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 4eccff3969..bd61c9c62f 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -18,12 +18,17 @@
namespace wallet {
static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
- std::set<CTxDestination> address_set;
+ std::set<CScript> output_scripts;
if (by_label) {
// Get the set of addresses assigned to label
std::string label = LabelFromValue(params[0]);
- address_set = wallet.GetLabelAddresses(label);
+ for (const auto& address : wallet.GetLabelAddresses(label)) {
+ auto output_script{GetScriptForDestination(address)};
+ if (wallet.IsMine(output_script)) {
+ output_scripts.insert(output_script);
+ }
+ }
} else {
// Get the address
CTxDestination dest = DecodeDestination(params[0].get_str());
@@ -34,7 +39,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
if (!wallet.IsMine(script_pub_key)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Address not found in wallet");
}
- address_set.insert(dest);
+ output_scripts.insert(script_pub_key);
}
// Minimum confirmations
@@ -66,8 +71,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
}
for (const CTxOut& txout : wtx.tx->vout) {
- CTxDestination address;
- if (ExtractDestination(txout.scriptPubKey, address) && wallet.IsMine(address) && address_set.count(address)) {
+ if (output_scripts.count(txout.scriptPubKey) > 0) {
amount += txout.nValue;
}
}
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/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index c87af2ea30..1b06973f78 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -15,6 +15,7 @@ using interfaces::FoundBlock;
namespace wallet {
static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue& entry)
+ EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
interfaces::Chain& chain = wallet.chain();
int confirms = wallet.GetTxDepthInMainChain(wtx);
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 55c0a2cb7f..e5fd7b0eb4 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -656,12 +656,10 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng
}
}
-static bool CreateTransactionInternal(
+static std::optional<CreatedTransactionResult> CreateTransactionInternal(
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,
@@ -669,6 +667,11 @@ static bool CreateTransactionInternal(
{
AssertLockHeld(wallet.cs_wallet);
+ // out variables, to be packed into returned result structure
+ CTransactionRef tx;
+ CAmount nFeeRet;
+ int nChangePosInOut = change_pos;
+
FastRandomContext rng_fast;
CMutableTransaction txNew; // The resulting transaction that we make
@@ -742,12 +745,12 @@ static bool CreateTransactionInternal(
// provided one
if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) {
error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB));
- return false;
+ return std::nullopt;
}
if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) {
// eventually allow a fallback fee
error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
- return false;
+ return std::nullopt;
}
// Calculate the cost of change
@@ -774,7 +777,7 @@ static bool CreateTransactionInternal(
if (IsDust(txout, wallet.chain().relayDustFee()))
{
error = _("Transaction amount too small");
- return false;
+ return std::nullopt;
}
txNew.vout.push_back(txout);
}
@@ -791,7 +794,7 @@ static bool CreateTransactionInternal(
std::optional<SelectionResult> result = SelectCoins(wallet, vAvailableCoins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
if (!result) {
error = _("Insufficient funds");
- return false;
+ return std::nullopt;
}
TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue());
@@ -808,7 +811,7 @@ static bool CreateTransactionInternal(
else if ((unsigned int)nChangePosInOut > txNew.vout.size())
{
error = _("Transaction change output index out of range");
- return false;
+ return std::nullopt;
}
assert(nChangePosInOut != -1);
@@ -836,7 +839,7 @@ static bool CreateTransactionInternal(
int nBytes = tx_sizes.vsize;
if (nBytes == -1) {
error = _("Missing solving data for estimating transaction size");
- return false;
+ return std::nullopt;
}
nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes);
@@ -900,7 +903,7 @@ static bool CreateTransactionInternal(
} else {
error = _("The transaction amount is too small to send after the fee has been deducted");
}
- return false;
+ return std::nullopt;
}
}
++i;
@@ -910,12 +913,12 @@ static bool CreateTransactionInternal(
// Give up if change keypool ran out and change is required
if (scriptChange.empty() && nChangePosInOut != -1) {
- return false;
+ return std::nullopt;
}
if (sign && !wallet.SignTransaction(txNew)) {
error = _("Signing transaction failed");
- return false;
+ return std::nullopt;
}
// Return the constructed transaction data.
@@ -926,19 +929,19 @@ static bool CreateTransactionInternal(
(!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT))
{
error = _("Transaction too large");
- return false;
+ return std::nullopt;
}
if (nFeeRet > wallet.m_default_max_tx_fee) {
error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED);
- return false;
+ return std::nullopt;
}
if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
// Lastly, ensure this tx will pass the mempool's chain limits
if (!wallet.chain().checkChainLimits(tx)) {
error = _("Transaction has too long of a mempool chain");
- return false;
+ return std::nullopt;
}
}
@@ -955,15 +958,13 @@ static bool CreateTransactionInternal(
feeCalc.est.fail.start, feeCalc.est.fail.end,
(feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0,
feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool);
- return true;
+ return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut);
}
-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,
@@ -971,42 +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)
- bool res = CreateTransactionInternal(wallet, vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign);
- 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
- if (CreateTransactionInternal(wallet, vecSend, tx2, nFeeRet2, 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) {
// 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)
@@ -1030,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 e43aac5273..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
@@ -82,12 +84,22 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control,
const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+struct CreatedTransactionResult
+{
+ CTransactionRef tx;
+ CAmount fee;
+ int change_pos;
+
+ CreatedTransactionResult(CTransactionRef tx, CAmount fee, int change_pos)
+ : tx(tx), fee(fee), change_pos(change_pos) {}
+};
+
/**
* 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;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 5a23f739d3..6c333c709b 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1841,6 +1841,8 @@ void CWallet::ReacceptWalletTransactions()
bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
{
+ AssertLockHeld(cs_wallet);
+
// Can't relay if wallet is not broadcasting
if (!GetBroadcastTransactions()) return false;
// Don't relay abandoned transactions
@@ -1869,12 +1871,11 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string
std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const
{
- std::set<uint256> result;
- {
- uint256 myHash = wtx.GetHash();
- result = GetConflicts(myHash);
- result.erase(myHash);
- }
+ AssertLockHeld(cs_wallet);
+
+ const uint256 myHash{wtx.GetHash()};
+ std::set<uint256> result{GetConflicts(myHash)};
+ result.erase(myHash);
return result;
}
@@ -2964,6 +2965,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
// so that in case of a shutdown event, the rescan will be repeated at the next start.
// This is temporary until rescan and notifications delivery are unified under same
// interface.
+ walletInstance->m_attaching_chain = true; //ignores chainStateFlushed notifications
walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance);
// If rescan_required = true, rescan_height remains equal to 0
@@ -2990,7 +2992,6 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
if (tip_height && *tip_height != rescan_height)
{
- walletInstance->m_attaching_chain = true; //ignores chainStateFlushed notifications
if (chain.havePruned()) {
int block_height = *tip_height;
while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) {
@@ -3034,6 +3035,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
walletInstance->chainStateFlushed(chain.getTipLocator());
walletInstance->GetDatabase().IncrementUpdateCounter();
}
+ walletInstance->m_attaching_chain = false;
return true;
}
@@ -3127,8 +3129,11 @@ int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const
int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const
{
- if (!wtx.IsCoinBase())
+ AssertLockHeld(cs_wallet);
+
+ if (!wtx.IsCoinBase()) {
return 0;
+ }
int chain_depth = GetTxDepthInMainChain(wtx);
assert(chain_depth >= 0); // coinbase tx should not be conflicted
return std::max(0, (COINBASE_MATURITY+1) - chain_depth);
@@ -3136,6 +3141,8 @@ int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const
bool CWallet::IsTxImmatureCoinBase(const CWalletTx& wtx) const
{
+ AssertLockHeld(cs_wallet);
+
// note GetBlocksToMaturity is 0 for non-coinbase tx
return GetTxBlocksToMaturity(wtx) > 0;
}
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 4e81a2b957..7da601c3b7 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -415,13 +415,7 @@ public:
const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
- // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
- // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to
- // resolve the issue of member access into incomplete type CWallet. Note
- // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)"
- // in place.
- std::set<uint256> GetTxConflicts(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS;
+ std::set<uint256> GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/**
* Return depth of transaction in blockchain:
@@ -429,22 +423,20 @@ public:
* 0 : in memory pool, waiting to be included in a block
* >=1 : this many blocks deep in the main chain
*/
- // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
- // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
- // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to
- // resolve the issue of member access into incomplete type CWallet. Note
- // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)"
- // in place.
- int GetTxDepthInMainChain(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS;
- bool IsTxInMainChain(const CWalletTx& wtx) const { return GetTxDepthInMainChain(wtx) > 0; }
+ int GetTxDepthInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ bool IsTxInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)
+ {
+ AssertLockHeld(cs_wallet);
+ return GetTxDepthInMainChain(wtx) > 0;
+ }
/**
* @return number of blocks to maturity for this transaction:
* 0 : is not a coinbase transaction, or is a mature coinbase transaction
* >0 : is a coinbase transaction which matures in this many blocks
*/
- int GetTxBlocksToMaturity(const CWalletTx& wtx) const;
- bool IsTxImmatureCoinBase(const CWalletTx& wtx) const;
+ int GetTxBlocksToMaturity(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ bool IsTxImmatureCoinBase(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
//! check whether we support the named feature
bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); }
@@ -584,7 +576,8 @@ public:
void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm);
/** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */
- bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const;
+ bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
+ EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const
{
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 20:35:21 +0000