← Watch feed
wallet: Make CWalletTx::tx private and use CWalletTx::GetTx to access
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Ava Chow
wallet: Make CWalletTx::tx private and use CWalletTx::GetTx to access
When CWalletTx will have multiple transactions, tx will no longer exist
and accessing the single canonical tx should be done through an getter
function.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a routine internal code cleanup in Bitcoin Core's wallet module. It makes the transaction pointer inside a wallet transaction object private and forces the rest of the code to read it through a getter function. There is no security fix here; the change prepares the code for a future feature where a wallet transaction might hold more than one transaction.
Recommended action
No security action required. Treat as normal refactoring. Reviewers may optionally verify that all wtx.tx accesses were replaced and that GetTx() returns the same reference previously exposed directly.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors CWalletTx so that its public CTransactionRef tx member becomes private, accessed via a new const getter CWalletTx::GetTx(). All call sites across the wallet, RPC, benchmarks, and tests are mechanically updated from wtx.tx to wtx.GetTx(). The constructor initializer order is adjusted accordingly. The stated rationale is future-proofing: when CWalletTx supports multiple transactions, the single canonical transaction will no longer be a direct member and must be retrieved through a getter. No behavior changes, bounds checks, or validation logic are introduced or removed.
Changed components
src/wallet/transaction.hsrc/wallet/transaction.cppsrc/wallet/wallet.cppsrc/wallet/spend.cppsrc/wallet/receive.cppsrc/wallet/feebumper.cppsrc/wallet/interfaces.cppsrc/wallet/rpc/coins.cppsrc/wallet/rpc/spend.cppsrc/wallet/rpc/transactions.cppsrc/bench/coin_selection.cppsrc/wallet/test/coinselector_tests.cppsrc/wallet/test/group_outputs_tests.cppsrc/wallet/test/wallet_tests.cppInspect captured patch +108 / −103
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index 5c54cafc..396d21d4 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -76,7 +76,7 @@ static void CoinSelection(benchmark::Bench& bench)
// Create coins from the amounts assigning them various output types
wallet::CoinsResult available_coins;
for (const auto& wtx : wtxs) {
- const auto txout = wtx->tx->vout.at(0);
+ const auto txout = wtx->GetTx()->vout.at(0);
OutputType outtype;
int input_bytes;
int y{det_rand.randrange(100)};
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 19157e2e..c3e422b3 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -24,7 +24,7 @@ namespace wallet {
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
- if (wallet.HasWalletSpend(wtx.tx)) {
+ if (wallet.HasWalletSpend(wtx.GetTx())) {
errors.emplace_back(Untranslated("Transaction has descendants in the wallet"));
return feebumper::Result::INVALID_PARAMETER;
}
@@ -49,7 +49,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
if (require_mine) {
// check that original tx consists entirely of our inputs
// if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
- if (!AllInputsMine(wallet, *wtx.tx)) {
+ if (!AllInputsMine(wallet, *wtx.GetTx())) {
errors.emplace_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
return feebumper::Result::WALLET_ERROR;
}
@@ -123,7 +123,7 @@ static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, con
// Get the fee rate of the original transaction. This is calculated from
// the tx fee/vsize, so it may have been rounded down. Add 1 satoshi to the
// result.
- int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
+ int64_t txSize = GetVirtualTransactionSize(*(wtx.GetTx()));
CFeeRate feerate(old_fee, txSize);
feerate += CFeeRate(1);
@@ -178,9 +178,10 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
return Result::INVALID_ADDRESS_OR_KEY;
}
const CWalletTx& wtx = it->second;
+ const CTransactionRef& tx = wtx.GetTx();
// Make sure that original_change_index is valid
- if (original_change_index.has_value() && original_change_index.value() >= wtx.tx->vout.size()) {
+ if (original_change_index.has_value() && original_change_index.value() >= tx->vout.size()) {
errors.emplace_back(Untranslated("Change position is out of range"));
return Result::INVALID_PARAMETER;
}
@@ -190,11 +191,11 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
std::map<COutPoint, Coin> coins;
CAmount input_value = 0;
std::vector<CTxOut> spent_outputs;
- for (const CTxIn& txin : wtx.tx->vin) {
+ for (const CTxIn& txin : tx->vin) {
coins[txin.prevout]; // Create empty map entry keyed by prevout.
}
wallet.chain().findCoins(coins);
- for (const CTxIn& txin : wtx.tx->vin) {
+ for (const CTxIn& txin : tx->vin) {
const Coin& coin = coins.at(txin.prevout);
if (coin.out.IsNull()) {
errors.emplace_back(Untranslated(strprintf("%s:%u is already spent", txin.prevout.hash.GetHex(), txin.prevout.n)));
@@ -210,9 +211,9 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// Figure out if we need to compute the input weight, and do so if necessary
PrecomputedTransactionData txdata;
- txdata.Init(*wtx.tx, std::move(spent_outputs), /* force=*/ true);
- for (unsigned int i = 0; i < wtx.tx->vin.size(); ++i) {
- const CTxIn& txin = wtx.tx->vin.at(i);
+ txdata.Init(*tx, std::move(spent_outputs), /* force=*/ true);
+ for (unsigned int i = 0; i < tx->vin.size(); ++i) {
+ const CTxIn& txin = tx->vin.at(i);
const Coin& coin = coins.at(txin.prevout);
if (new_coin_control.IsExternalSelected(txin.prevout)) {
@@ -223,7 +224,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// In order to do this, we verify the script with a special SignatureChecker which
// will observe the signatures verified and record their sizes.
SignatureWeights weights;
- TransactionSignatureChecker tx_checker(wtx.tx.get(), i, coin.out.nValue, txdata, MissingDataBehavior::FAIL);
+ TransactionSignatureChecker tx_checker(tx.get(), i, coin.out.nValue, txdata, MissingDataBehavior::FAIL);
SignatureWeightChecker size_checker(weights, tx_checker);
VerifyScript(txin.scriptSig, coin.out.scriptPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, size_checker);
// Add the difference between max and current to input_weight so that it represents the largest the input could be
@@ -239,7 +240,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// Calculate the old output amount.
CAmount output_value = 0;
- for (const auto& old_output : wtx.tx->vout) {
+ for (const auto& old_output : tx->vout) {
output_value += old_output.nValue;
}
@@ -250,7 +251,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// outputs with its contents, otherwise use original outputs.
std::vector<CRecipient> recipients;
CAmount new_outputs_value = 0;
- const auto& txouts = outputs.empty() ? wtx.tx->vout : outputs;
+ const auto& txouts = outputs.empty() ? tx->vout : outputs;
for (size_t i = 0; i < txouts.size(); ++i) {
const CTxOut& output = txouts.at(i);
CTxDestination dest;
@@ -282,7 +283,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// The user provided a feeRate argument.
// We calculate this here to avoid compiler warning on the cs_wallet lock
// We need to make a temporary transaction with no input witnesses as the dummy signer expects them to be empty for external inputs
- CMutableTransaction temp_mtx{*wtx.tx};
+ CMutableTransaction temp_mtx{*tx};
for (auto& txin : temp_mtx.vin) {
txin.scriptSig.clear();
txin.scriptWitness.SetNull();
@@ -305,7 +306,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const Txid& txid, const CCoinC
// A2 and A3 where A2 and A3 don't conflict (or alternatively bump A to A2 and A2
// to A3 where A and A3 don't conflict). If both later get confirmed then the sender
// has accidentally double paid.
- for (const auto& inputs : wtx.tx->vin) {
+ for (const auto& inputs : tx->vin) {
new_coin_control.Select(COutPoint(inputs.prevout));
}
new_coin_control.m_allow_other_inputs = true;
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index f467f43d..24b63738 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -57,15 +57,15 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
{
LOCK(wallet.cs_wallet);
WalletTx result;
- result.tx = wtx.tx;
- result.txin_is_mine.reserve(wtx.tx->vin.size());
- for (const auto& txin : wtx.tx->vin) {
+ result.tx = wtx.GetTx();
+ result.txin_is_mine.reserve(result.tx->vin.size());
+ for (const auto& txin : result.tx->vin) {
result.txin_is_mine.emplace_back(InputIsMine(wallet, txin));
}
- result.txout_is_mine.reserve(wtx.tx->vout.size());
- result.txout_address.reserve(wtx.tx->vout.size());
- result.txout_address_is_mine.reserve(wtx.tx->vout.size());
- for (const auto& txout : wtx.tx->vout) {
+ result.txout_is_mine.reserve(result.tx->vout.size());
+ result.txout_address.reserve(result.tx->vout.size());
+ result.txout_address_is_mine.reserve(result.tx->vout.size());
+ for (const auto& txout : result.tx->vout) {
result.txout_is_mine.emplace_back(wallet.IsMine(txout));
result.txout_is_change.push_back(OutputIsChange(wallet, txout));
result.txout_address.emplace_back();
@@ -99,7 +99,7 @@ WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
result.blocks_to_maturity = wallet.GetTxBlocksToMaturity(wtx);
result.depth_in_main_chain = wallet.GetTxDepthInMainChain(wtx);
result.time_received = wtx.nTimeReceived;
- result.lock_time = wtx.tx->nLockTime;
+ result.lock_time = wtx.GetTx()->nLockTime;
result.is_trusted = CachedTxIsTrusted(wallet, wtx);
result.is_abandoned = wtx.isAbandoned();
result.is_coinbase = wtx.IsCoinBase();
@@ -114,7 +114,7 @@ WalletTxOut MakeWalletTxOut(const CWallet& wallet,
int depth) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
WalletTxOut result;
- result.txout = wtx.tx->vout[n];
+ result.txout = wtx.GetTx()->vout[n];
result.time = wtx.GetTxTime();
result.depth_in_main_chain = depth;
result.is_spent = wallet.IsSpent(COutPoint(wtx.GetHash(), n));
@@ -305,7 +305,7 @@ public:
LOCK(m_wallet->cs_wallet);
auto mi = m_wallet->mapWallet.find(txid);
if (mi != m_wallet->mapWallet.end()) {
- return mi->second.tx;
+ return mi->second.GetTx();
}
return {};
}
diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp
index 86bc63f8..4559eb9d 100644
--- a/src/wallet/receive.cpp
+++ b/src/wallet/receive.cpp
@@ -14,8 +14,8 @@ bool InputIsMine(const CWallet& wallet, const CTxIn& txin)
{
AssertLockHeld(wallet.cs_wallet);
const CWalletTx* prev = wallet.GetWalletTx(txin.prevout.hash);
- if (prev && txin.prevout.n < prev->tx->vout.size()) {
- return wallet.IsMine(prev->tx->vout[txin.prevout.n]);
+ if (prev && txin.prevout.n < prev->GetTx()->vout.size()) {
+ return wallet.IsMine(prev->GetTx()->vout[txin.prevout.n]);
}
return false;
}
@@ -101,7 +101,7 @@ static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CW
{
auto& amount = wtx.m_amounts[type];
if (!amount.IsCached(avoid_reuse)) {
- amount.Set(avoid_reuse, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx) : TxGetCredit(wallet, *wtx.tx));
+ amount.Set(avoid_reuse, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.GetTx()) : TxGetCredit(wallet, *wtx.GetTx()));
wtx.m_is_cache_empty = false;
}
return amount.Get(avoid_reuse);
@@ -121,7 +121,7 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, bool avoi
CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, bool avoid_reuse)
{
- if (wtx.tx->vin.empty())
+ if (wtx.GetTx()->vin.empty())
return 0;
return GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, avoid_reuse);
@@ -131,7 +131,7 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx)
{
if (wtx.fChangeCached)
return wtx.nChangeCached;
- wtx.nChangeCached = TxGetChange(wallet, *wtx.tx);
+ wtx.nChangeCached = TxGetChange(wallet, *wtx.GetTx());
wtx.fChangeCached = true;
return wtx.nChangeCached;
}
@@ -149,15 +149,15 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
CAmount nDebit = CachedTxGetDebit(wallet, wtx, /*avoid_reuse=*/false);
if (nDebit > 0) // debit>0 means we signed/sent this transaction
{
- CAmount nValueOut = wtx.tx->GetValueOut();
+ CAmount nValueOut = wtx.GetTx()->GetValueOut();
nFee = nDebit - nValueOut;
}
LOCK(wallet.cs_wallet);
// Sent/received.
- for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i)
+ for (unsigned int i = 0; i < wtx.GetTx()->vout.size(); ++i)
{
- const CTxOut& txout = wtx.tx->vout[i];
+ const CTxOut& txout = wtx.GetTx()->vout[i];
bool ismine = wallet.IsMine(txout);
// Only need to handle txouts if AT LEAST one of these is true:
// 1) they debit from us (sent)
@@ -196,7 +196,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx)
{
if (!wtx.m_cached_from_me.has_value()) {
- wtx.m_cached_from_me = wallet.IsFromMe(*wtx.tx);
+ wtx.m_cached_from_me = wallet.IsFromMe(*wtx.GetTx());
}
return wtx.m_cached_from_me.value();
}
@@ -218,12 +218,12 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<Txi
if (!wtx.InMempool()) return false;
// Trusted if all inputs are from us and are in the mempool:
- for (const CTxIn& txin : wtx.tx->vin)
+ for (const CTxIn& txin : wtx.GetTx()->vin)
{
// Transactions not sent by us: not trusted
const CWalletTx* parent = wallet.GetWalletTx(txin.prevout.hash);
if (parent == nullptr) return false;
- const CTxOut& parentOut = parent->tx->vout[txin.prevout.n];
+ const CTxOut& parentOut = parent->GetTx()->vout[txin.prevout.n];
// Check that this specific input being spent is trusted
if (!wallet.IsMine(parentOut)) return false;
// If we've already trusted this parent, continue
@@ -332,16 +332,16 @@ std::set< std::set<CTxDestination> > GetAddressGroupings(const CWallet& wallet)
{
const CWalletTx& wtx = walletEntry.second;
- if (wtx.tx->vin.size() > 0)
+ if (wtx.GetTx()->vin.size() > 0)
{
bool any_mine = false;
// group all input addresses with each other
- for (const CTxIn& txin : wtx.tx->vin)
+ for (const CTxIn& txin : wtx.GetTx()->vin)
{
CTxDestination address;
if(!InputIsMine(wallet, txin)) /* If this input isn't mine, ignore it */
continue;
- if(!ExtractDestination(wallet.mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address))
+ if(!ExtractDestination(wallet.mapWallet.at(txin.prevout.hash).GetTx()->vout[txin.prevout.n].scriptPubKey, address))
continue;
grouping.insert(address);
any_mine = true;
@@ -350,7 +350,7 @@ std::set< std::set<CTxDestination> > GetAddressGroupings(const CWallet& wallet)
// group change with input addresses
if (any_mine)
{
- for (const CTxOut& txout : wtx.tx->vout)
+ for (const CTxOut& txout : wtx.GetTx()->vout)
if (OutputIsChange(wallet, txout))
{
CTxDestination txoutAddr;
@@ -367,7 +367,7 @@ std::set< std::set<CTxDestination> > GetAddressGroupings(const CWallet& wallet)
}
// group lone addrs by themselves
- for (const auto& txout : wtx.tx->vout)
+ for (const auto& txout : wtx.GetTx()->vout)
if (wallet.IsMine(txout))
{
CTxDestination address;
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index ab869b0d..96dfefad 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -66,7 +66,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
continue;
}
- for (const CTxOut& txout : wtx.tx->vout) {
+ for (const CTxOut& txout : wtx.GetTx()->vout) {
if (output_scripts.contains(txout.scriptPubKey)) {
amount += txout.nValue;
}
@@ -309,7 +309,7 @@ RPCMethod lockunspent()
const CWalletTx& trans = it->second;
- if (outpt.n >= trans.tx->vout.size()) {
+ if (outpt.n >= trans.GetTx()->vout.size()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout index out of bounds");
}
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 65967989..ac5c52d2 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1478,17 +1478,17 @@ RPCMethod sendall()
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not available. UTXO (%s:%d) was already spent.", input.prevout.hash.ToString(), input.prevout.n));
}
const CWalletTx* tx{pwallet->GetWalletTx(input.prevout.hash)};
- if (!tx || input.prevout.n >= tx->tx->vout.size() || !pwallet->IsMine(tx->tx->vout[input.prevout.n])) {
+ if (!tx || input.prevout.n >= tx->GetTx()->vout.size() || !pwallet->IsMine(tx->GetTx()->vout[input.prevout.n])) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input not found. UTXO (%s:%d) is not part of wallet.", input.prevout.hash.ToString(), input.prevout.n));
}
if (pwallet->GetTxDepthInMainChain(*tx) == 0) {
- if (tx->tx->version == TRUC_VERSION && coin_control.m_version != TRUC_VERSION) {
+ if (tx->GetTx()->version == TRUC_VERSION && coin_control.m_version != TRUC_VERSION) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Can't spend unconfirmed version 3 pre-selected input with a version %d tx", coin_control.m_version));
- } else if (coin_control.m_version == TRUC_VERSION && tx->tx->version != TRUC_VERSION) {
- throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Can't spend unconfirmed version %d pre-selected input with a version 3 tx", tx->tx->version));
+ } else if (coin_control.m_version == TRUC_VERSION && tx->GetTx()->version != TRUC_VERSION) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Can't spend unconfirmed version %d pre-selected input with a version 3 tx", tx->GetTx()->version));
}
}
- total_input_value += tx->tx->vout[input.prevout.n].nValue;
+ total_input_value += tx->GetTx()->vout[input.prevout.n].nValue;
}
} else {
CoinFilterParams coins_params;
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index 480a346f..635f150b 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -53,7 +53,7 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue
if (chain.rpcEnableDeprecated("bip125")) {
std::string rbfStatus = "no";
if (confirms <= 0) {
- RBFTransactionState rbfState = chain.isRBFOptIn(*wtx.tx);
+ RBFTransactionState rbfState = chain.isRBFOptIn(*wtx.GetTx());
if (rbfState == RBFTransactionState::UNKNOWN)
rbfStatus = "unknown";
else if (rbfState == RBFTransactionState::REPLACEABLE_BIP125)
@@ -110,7 +110,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
continue;
}
- for (const CTxOut& txout : wtx.tx->vout) {
+ for (const CTxOut& txout : wtx.GetTx()->vout) {
CTxDestination address;
if (!ExtractDestination(txout.scriptPubKey, address))
continue;
@@ -354,7 +354,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM
}
UniValue entry(UniValue::VOBJ);
MaybePushAddress(entry, r.destination);
- PushParentDescriptors(wallet, wtx.tx->vout.at(r.vout).scriptPubKey, entry);
+ PushParentDescriptors(wallet, wtx.GetTx()->vout.at(r.vout).scriptPubKey, entry);
if (wtx.IsCoinBase())
{
if (wallet.GetTxDepthInMainChain(wtx) < 1)
@@ -755,7 +755,7 @@ RPCMethod gettransaction()
CAmount nCredit = CachedTxGetCredit(*pwallet, wtx, /*avoid_reuse=*/false);
CAmount nDebit = CachedTxGetDebit(*pwallet, wtx, /*avoid_reuse=*/false);
CAmount nNet = nCredit - nDebit;
- CAmount nFee = (CachedTxIsFromMe(*pwallet, wtx) ? wtx.tx->GetValueOut() - nDebit : 0);
+ CAmount nFee = (CachedTxIsFromMe(*pwallet, wtx) ? wtx.GetTx()->GetValueOut() - nDebit : 0);
entry.pushKV("amount", ValueFromAmount(nNet - nFee));
if (CachedTxIsFromMe(*pwallet, wtx))
@@ -767,11 +767,11 @@ RPCMethod gettransaction()
ListTransactions(*pwallet, wtx, 0, false, details, /*filter_label=*/std::nullopt);
entry.pushKV("details", std::move(details));
- entry.pushKV("hex", EncodeHexTx(*wtx.tx));
+ entry.pushKV("hex", EncodeHexTx(*wtx.GetTx()));
if (verbose) {
UniValue decoded(UniValue::VOBJ);
- TxToUniv(*wtx.tx,
+ TxToUniv(*wtx.GetTx(),
/*block_hash=*/uint256(),
/*entry=*/decoded,
/*include_hex=*/false,
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 51247b55..1e491ee2 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -178,8 +178,8 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle
const auto mi = wallet->mapWallet.find(input.prevout.hash);
// Can not estimate size without knowing the input details
if (mi != wallet->mapWallet.end()) {
- assert(input.prevout.n < mi->second.tx->vout.size());
- txouts.emplace_back(mi->second.tx->vout.at(input.prevout.n));
+ assert(input.prevout.n < mi->second.GetTx()->vout.size());
+ txouts.emplace_back(mi->second.GetTx()->vout.at(input.prevout.n));
} else if (coin_control) {
const auto& txout{coin_control->GetExternalOutput(input.prevout)};
if (!txout) return TxSize{-1, -1};
@@ -281,10 +281,10 @@ util::Result<CoinsResult> FetchSelectedInputs(const CWallet& wallet, const CCoin
}
const CWalletTx& parent_tx = txo->GetWalletTx();
if (wallet.GetTxDepthInMainChain(parent_tx) == 0) {
- if (parent_tx.tx->version == TRUC_VERSION && coin_control.m_version != TRUC_VERSION) {
+ if (parent_tx.GetTx()->version == TRUC_VERSION && coin_control.m_version != TRUC_VERSION) {
return util::Error{strprintf(_("Can't spend unconfirmed version 3 pre-selected input with a version %d tx"), coin_control.m_version)};
- } else if (coin_control.m_version == TRUC_VERSION && parent_tx.tx->version != TRUC_VERSION) {
- return util::Error{strprintf(_("Can't spend unconfirmed version %d pre-selected input with a version 3 tx"), parent_tx.tx->version)};
+ } else if (coin_control.m_version == TRUC_VERSION && parent_tx.GetTx()->version != TRUC_VERSION) {
+ return util::Error{strprintf(_("Can't spend unconfirmed version %d pre-selected input with a version 3 tx"), parent_tx.GetTx()->version)};
}
}
} else {
@@ -396,16 +396,16 @@ CoinsResult AvailableCoins(const CWallet& wallet,
if (nDepth == 0 && params.check_version_trucness) {
if (coinControl->m_version == TRUC_VERSION) {
- if (wtx.tx->version != TRUC_VERSION) continue;
+ if (wtx.GetTx()->version != TRUC_VERSION) continue;
// this unconfirmed v3 transaction already has a child
if (wtx.truc_child_in_mempool.has_value()) continue;
// this unconfirmed v3 transaction has a parent: spending would create a third generation
size_t ancestors, unused_cluster_count;
- wallet.chain().getTransactionAncestry(wtx.tx->GetHash(), ancestors, unused_cluster_count);
+ wallet.chain().getTransactionAncestry(wtx.GetTx()->GetHash(), ancestors, unused_cluster_count);
if (ancestors > 1) continue;
} else {
- if (wtx.tx->version == TRUC_VERSION) continue;
+ if (wtx.GetTx()->version == TRUC_VERSION) continue;
}
}
@@ -468,9 +468,9 @@ CoinsResult AvailableCoins(const CWallet& wallet,
auto available_output_type = GetOutputType(type, is_from_p2sh);
auto available_output = COutput(outpoint, output, nDepth, input_bytes, solvable, tx_safe, wtx.GetTxTime(), tx_from_me, feerate);
- if (wtx.tx->version == TRUC_VERSION && nDepth == 0 && params.check_version_trucness) {
+ if (wtx.GetTx()->version == TRUC_VERSION && nDepth == 0 && params.check_version_trucness) {
unconfirmed_truc_coins.emplace_back(available_output_type, available_output);
- auto [it, _] = truc_txid_by_value.try_emplace(wtx.tx->GetHash(), 0);
+ auto [it, _] = truc_txid_by_value.try_emplace(wtx.GetTx()->GetHash(), 0);
it->second += output.nValue;
} else {
result.Add(available_output_type, available_output);
@@ -526,16 +526,16 @@ const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint&
AssertLockHeld(wallet.cs_wallet);
const CWalletTx* wtx{Assert(wallet.GetWalletTx(outpoint.hash))};
- const CTransaction* ptx = wtx->tx.get();
+ const CTransaction* ptx = wtx->GetTx().get();
int n = outpoint.n;
while (OutputIsChange(wallet, ptx->vout[n]) && ptx->vin.size() > 0) {
const COutPoint& prevout = ptx->vin[0].prevout;
const CWalletTx* it = wallet.GetWalletTx(prevout.hash);
- if (!it || it->tx->vout.size() <= prevout.n ||
- !wallet.IsMine(it->tx->vout[prevout.n])) {
+ if (!it || it->GetTx()->vout.size() <= prevout.n ||
+ !wallet.IsMine(it->GetTx()->vout[prevout.n])) {
break;
}
- ptx = it->tx.get();
+ ptx = it->GetTx().get();
n = prevout.n;
}
return ptx->vout[n];
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 2d493795..dee4b05c 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -76,7 +76,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun
auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{}));
assert(ret.second);
CWalletTx& wtx = (*ret.first).second;
- const auto& txout = wtx.tx->vout.at(nInput);
+ const auto& txout = wtx.GetTx()->vout.at(nInput);
available_coins.Add(OutputType::BECH32, {COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*solvable=*/true, /*safe=*/true, wtx.GetTxTime(), fIsFromMe, feerate});
}
diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp
index 4bbf145c..9b4d9bc8 100644
--- a/src/wallet/test/group_outputs_tests.cpp
+++ b/src/wallet/test/group_outputs_tests.cpp
@@ -44,7 +44,7 @@ static void addCoin(CoinsResult& coins,
auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{}));
assert(ret.second);
CWalletTx& wtx = (*ret.first).second;
- const auto& txout = wtx.tx->vout.at(0);
+ const auto& txout = wtx.GetTx()->vout.at(0);
coins.Add(*Assert(OutputTypeFromDestination(dest)),
{COutPoint(wtx.GetHash(), 0),
txout,
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 49824c80..c7ce9553 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -406,7 +406,7 @@ public:
CMutableTransaction blocktx;
{
LOCK(wallet->cs_wallet);
- blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx);
+ blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).GetTx());
}
CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
@@ -489,7 +489,7 @@ void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount
filter.skip_locked = false;
CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter);
// Lock outputs so they are not spent in follow-up transactions
- for (uint32_t i = 0; i < wtx.tx->vout.size(); i++) context.wallet->LockCoin({wtx.GetHash(), i}, /*persist=*/false);
+ for (uint32_t i = 0; i < wtx.GetTx()->vout.size(); i++) context.wallet->LockCoin({wtx.GetHash(), i}, /*persist=*/false);
for (const auto& [type, size] : expected_coins_sizes) BOOST_CHECK_EQUAL(size, available_coins.coins[type].size());
}
diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp
index f1bf62aa..9779fe47 100644
--- a/src/wallet/transaction.cpp
+++ b/src/wallet/transaction.cpp
@@ -11,8 +11,8 @@ using interfaces::FoundBlock;
namespace wallet {
bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
{
- CMutableTransaction tx1 {*this->tx};
- CMutableTransaction tx2 {*_tx.tx};
+ CMutableTransaction tx1 {*this->GetTx()};
+ CMutableTransaction tx2 {*_tx.GetTx()};
for (auto& txin : tx1.vin) {
txin.scriptSig = CScript();
txin.scriptWitness.SetNull();
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index c65d2c69..8f1806bf 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -234,7 +234,7 @@ public:
mutable bool fChangeCached;
mutable CAmount nChangeCached;
- CWalletTx(CTransactionRef tx, const TxState& state) : tx(std::move(Assert(tx))), m_state(state)
+ CWalletTx(CTransactionRef tx, const TxState& state) : m_state(state), tx(std::move(Assert(tx)))
{
Init();
}
@@ -254,7 +254,6 @@ public:
nOrderPos = -1;
}
- CTransactionRef tx;
TxState m_state;
// Set of mempool transactions that conflict
@@ -343,6 +342,8 @@ public:
}
}
+ CTransactionRef GetTx() const { return tx; }
+
void SetTx(CTransactionRef arg)
{
tx = std::move(arg);
@@ -390,6 +391,9 @@ public:
// Enable the default move constructor
CWalletTx(CWalletTx&&) = default;
+
+private:
+ CTransactionRef tx;
};
struct WalletTxOrderComparator {
@@ -410,7 +414,7 @@ public:
: m_wtx(wtx),
m_output(output)
{
- Assume(std::ranges::find(wtx.tx->vout, output) != wtx.tx->vout.end());
+ Assume(std::ranges::find(wtx.GetTx()->vout, output) != wtx.GetTx()->vout.end());
}
const CWalletTx& GetWalletTx() const { return m_wtx; }
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 25510c5f..54233ea5 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -693,7 +693,7 @@ std::set<Txid> CWallet::GetConflicts(const Txid& txid) const
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range;
- for (const CTxIn& txin : wtx.tx->vin)
+ for (const CTxIn& txin : wtx.GetTx()->vin)
{
if (mapTxSpends.count(txin.prevout) <= 1)
continue; // No conflict if zero or one spends
@@ -825,7 +825,7 @@ void CWallet::AddToSpends(const CWalletTx& wtx)
if (wtx.IsCoinBase()) // Coinbases don't spend anything!
return;
- for (const CTxIn& txin : wtx.tx->vin)
+ for (const CTxIn& txin : wtx.GetTx()->vin)
AddToSpends(txin.prevout, wtx.GetHash());
}
@@ -1021,7 +1021,7 @@ void CWallet::SetSpentKeyState(WalletBatch& batch, const Txid& hash, unsigned in
if (!srctx) return;
CTxDestination dst;
- if (ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst)) {
+ if (ExtractDestination(srctx->GetTx()->vout[n].scriptPubKey, dst)) {
if (IsMine(dst)) {
if (used != IsAddressPreviouslySpent(dst)) {
if (used) {
@@ -1096,7 +1096,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
// wallet. Store the new version of the transaction with the witness,
// as the stripped-version must be invalid.
// TODO: Store all versions of the transaction, instead of just one.
- if (tx->HasWitness() && !wtx.tx->HasWitness()) {
+ if (tx->HasWitness() && !wtx.GetTx()->HasWitness()) {
wtx.SetTx(tx);
fUpdated = true;
}
@@ -1115,8 +1115,8 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
// Break caches since we have changed the state
desc_tx->MarkDirty();
batch.WriteTx(*desc_tx);
- MarkInputsDirty(desc_tx->tx);
- for (unsigned int i = 0; i < desc_tx->tx->vout.size(); ++i) {
+ MarkInputsDirty(desc_tx->GetTx());
+ for (unsigned int i = 0; i < desc_tx->GetTx()->vout.size(); ++i) {
COutPoint outpoint(desc_tx->GetHash(), i);
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(outpoint);
for (TxSpends::const_iterator it = range.first; it != range.second; ++it) {
@@ -1195,7 +1195,7 @@ bool CWallet::LoadToWallet(CWalletTx&& wtx_in)
}
wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx));
AddToSpends(wtx);
- for (const CTxIn& txin : wtx.tx->vin) {
+ for (const CTxIn& txin : wtx.GetTx()->vin) {
auto it = mapWallet.find(txin.prevout.hash);
if (it != mapWallet.end()) {
CWalletTx& prevtx = it->second;
@@ -1289,8 +1289,8 @@ void CWallet::UpdateTrucSiblingConflicts(const CWalletTx& parent_wtx, const Txid
{
// Find all other txs in our wallet that spend utxos from this parent
// so that we can mark them as mempool-conflicted by this new tx.
- for (long unsigned int i = 0; i < parent_wtx.tx->vout.size(); i++) {
- for (auto range = mapTxSpends.equal_range(COutPoint(parent_wtx.tx->GetHash(), i)); range.first != range.second; range.first++) {
+ for (long unsigned int i = 0; i < parent_wtx.GetTx()->vout.size(); i++) {
+ for (auto range = mapTxSpends.equal_range(COutPoint(parent_wtx.GetTx()->GetHash(), i)); range.first != range.second; range.first++) {
const Txid& sibling_txid = range.first->second;
// Skip the child_tx itself
if (sibling_txid == child_txid) continue;
@@ -1404,7 +1404,7 @@ void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const Txid& tx_hash, co
wtx.MarkDirty();
if (batch) batch->WriteTx(wtx);
// Iterate over all its outputs, and update those tx states as well (if applicable)
- for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) {
+ for (unsigned int i = 0; i < wtx.GetTx()->vout.size(); ++i) {
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i));
for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) {
if (!done.contains(iter->second)) {
@@ -1419,7 +1419,7 @@ void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const Txid& tx_hash, co
// If a transaction changes its tx state, that usually changes the balance
// available of the outputs it spends. So force those to be recomputed
- MarkInputsDirty(wtx.tx);
+ MarkInputsDirty(wtx.GetTx());
}
}
}
@@ -1615,7 +1615,7 @@ void CWallet::blockDisconnected(const interfaces::BlockInfo& block)
return TxUpdate::UNCHANGED;
};
- RecursiveUpdateTxState(wtx.tx->GetHash(), try_updating_state);
+ RecursiveUpdateTxState(wtx.GetTx()->GetHash(), try_updating_state);
}
}
}
@@ -1697,10 +1697,10 @@ bool CWallet::IsMine(const COutPoint& outpoint) const
if (!wtx) {
return false;
}
- if (outpoint.n >= wtx->tx->vout.size()) {
+ if (outpoint.n >= wtx->GetTx()->vout.size()) {
return false;
}
- return IsMine(wtx->tx->vout[outpoint.n]);
+ return IsMine(wtx->GetTx()->vout[outpoint.n]);
}
bool CWallet::IsFromMe(const CTransaction& tx) const
@@ -2066,7 +2066,7 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx,
// If broadcast fails for any reason, trying to set wtx.m_state here would be incorrect.
// If transaction was previously in the mempool, it should be updated when
// TransactionRemovedFromMempool fires.
- bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, broadcast_method, err_string);
+ bool ret = chain().broadcastTransaction(wtx.GetTx(), m_default_max_tx_fee, broadcast_method, err_string);
if (ret) wtx.m_state = TxStateInMempool{};
return ret;
}
@@ -2180,12 +2180,12 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const
std::map<COutPoint, Coin> coins;
for (auto& input : tx.vin) {
const auto mi = mapWallet.find(input.prevout.hash);
- if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) {
+ if(mi == mapWallet.end() || input.prevout.n >= mi->second.GetTx()->vout.size()) {
return false;
}
const CWalletTx& wtx = mi->second;
int prev_height = wtx.state<TxStateConfirmed>() ? wtx.state<TxStateConfirmed>()->confirmed_block_height : 0;
- coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], prev_height, wtx.IsCoinBase());
+ coins[input.prevout] = Coin(wtx.GetTx()->vout[input.prevout.n], prev_height, wtx.IsCoinBase());
}
std::map<int, bilingual_str> input_errors;
return SignTransaction(tx, coins, SIGHASH_DEFAULT, input_errors);
@@ -2226,7 +2226,7 @@ std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, co
const CWalletTx& wtx = it->second;
// We only need the non_witness_utxo, which is a superset of the witness_utxo.
// The signing code will switch to the smaller witness_utxo if this is ok.
- input.non_witness_utxo = wtx.tx;
+ input.non_witness_utxo = wtx.GetTx();
}
}
}
@@ -2475,7 +2475,7 @@ util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs
for (const auto& it : erased_txs) {
const Txid hash{it->first};
wtxOrdered.erase(it->second.m_it_wtxOrdered);
- for (const auto& txin : it->second.tx->vin) {
+ for (const auto& txin : it->second.GetTx()->vin) {
auto range = mapTxSpends.equal_range(txin.prevout);
for (auto iter = range.first; iter != range.second; ++iter) {
if (iter->second == hash) {
@@ -2484,7 +2484,7 @@ util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs
}
}
}
- for (unsigned int i = 0; i < it->second.tx->vout.size(); ++i) {
+ for (unsigned int i = 0; i < it->second.GetTx()->vout.size(); ++i) {
m_txos.erase(COutPoint(hash, i));
}
mapWallet.erase(it);
@@ -2649,9 +2649,9 @@ void CWallet::MarkDestinationsDirty(const std::set<CTxDestination>& destinations
for (auto& entry : mapWallet) {
CWalletTx& wtx = entry.second;
if (wtx.m_is_cache_empty) continue;
- for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {
+ for (unsigned int i = 0; i < wtx.GetTx()->vout.size(); i++) {
CTxDestination dst;
- if (ExtractDestination(wtx.tx->vout[i].scriptPubKey, dst) && destinations.contains(dst)) {
+ if (ExtractDestination(wtx.GetTx()->vout[i].scriptPubKey, dst) && destinations.contains(dst)) {
wtx.MarkDirty();
break;
}
@@ -4041,10 +4041,10 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
for (const auto& [_pos, wtx] : wtxOrdered) {
// Check it is the watchonly wallet's
// solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for
- bool is_mine = IsMine(*wtx->tx) || IsFromMe(*wtx->tx);
+ bool is_mine = IsMine(*wtx->GetTx()) || IsFromMe(*wtx->GetTx());
if (data.watchonly_wallet) {
LOCK(data.watchonly_wallet->cs_wallet);
- if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) {
+ if (data.watchonly_wallet->IsMine(*wtx->GetTx()) || data.watchonly_wallet->IsFromMe(*wtx->GetTx())) {
// Add to watchonly wallet
const Txid& hash = wtx->GetHash();
DataStream wtx_ser;
@@ -4604,8 +4604,8 @@ void CWallet::WriteBestBlock() const
void CWallet::RefreshTXOsFromTx(const CWalletTx& wtx)
{
AssertLockHeld(cs_wallet);
- for (uint32_t i = 0; i < wtx.tx->vout.size(); ++i) {
- const CTxOut& txout = wtx.tx->vout.at(i);
+ for (uint32_t i = 0; i < wtx.GetTx()->vout.size(); ++i) {
+ const CTxOut& txout = wtx.GetTx()->vout.at(i);
if (!IsMine(txout)) continue;
COutPoint outpoint(wtx.GetHash(), i);
if (m_txos.contains(outpoint)) {
Risk score
Our methodology →Why this scored 19/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.