Skip to content

Commit e03fdff

Browse files
kwvgUdjinM6
kwvg
and
UdjinM6
committed
refactor: move ISLock and ChainLock wtx check to CWallet
Co-authored-by: UdjinM6 <[email protected]>
1 parent 460c187 commit e03fdff

File tree

6 files changed

+60
-38
lines changed

6 files changed

+60
-38
lines changed

‎src/wallet/interfaces.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
9494

9595
//! Construct wallet tx status struct.
9696
WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
97+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
9798
{
99+
AssertLockHeld(wallet.cs_wallet);
100+
98101
WalletTxStatus result;
99102
result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits<int>::max();
100103
result.blocks_to_maturity = wtx.GetBlocksToMaturity();
@@ -106,8 +109,8 @@ WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
106109
result.is_abandoned = wtx.isAbandoned();
107110
result.is_coinbase = wtx.IsCoinBase();
108111
result.is_in_main_chain = wtx.IsInMainChain();
109-
result.is_chainlocked = wtx.IsChainLocked();
110-
result.is_islocked = wtx.IsLockedByInstantSend();
112+
result.is_chainlocked = wallet.IsTxChainLocked(wtx);
113+
result.is_islocked = wallet.IsTxLockedByInstantSend(wtx);
111114
return result;
112115
}
113116

‎src/wallet/receive.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ bool CWallet::IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents
282282
int nDepth = wtx.GetDepthInMainChain();
283283
if (nDepth >= 1) return true;
284284
if (nDepth < 0) return false;
285-
if (wtx.IsLockedByInstantSend()) return true;
285+
if (IsTxLockedByInstantSend(wtx)) return true;
286286
// using wtx's cached debit
287287
if (!m_spend_zero_conf_change || !wtx.IsFromMe(ISMINE_ALL)) return false;
288288

@@ -329,7 +329,7 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, const bool avoid_reuse
329329
const int tx_depth{wtx.GetDepthInMainChain()};
330330
const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)};
331331
const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)};
332-
if (is_trusted && ((tx_depth >= min_depth) || (fAddLocked && wtx.IsLockedByInstantSend()))) {
332+
if (is_trusted && ((tx_depth >= min_depth) || (fAddLocked && IsTxLockedByInstantSend(wtx)))) {
333333
ret.m_mine_trusted += tx_credit_mine;
334334
ret.m_watchonly_trusted += tx_credit_watchonly;
335335
}
@@ -371,7 +371,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const
371371
continue;
372372

373373
int nDepth = wtx.GetDepthInMainChain();
374-
if ((nDepth < (wtx.IsFromMe(ISMINE_ALL) ? 0 : 1)) && !wtx.IsLockedByInstantSend())
374+
if ((nDepth < (wtx.IsFromMe(ISMINE_ALL) ? 0 : 1)) && !IsTxLockedByInstantSend(wtx))
375375
continue;
376376

377377
for (unsigned int i = 0; i < wtx.tx->vout.size(); i++)

‎src/wallet/rpcwallet.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,17 @@ bool HaveKey(const SigningProvider& wallet, const CKey& key)
5858
return wallet.HaveKey(key.GetPubKey().GetID()) || wallet.HaveKey(key2.GetPubKey().GetID());
5959
}
6060

61-
static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry)
61+
static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue& entry)
62+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
6263
{
64+
AssertLockHeld(wallet.cs_wallet);
65+
66+
interfaces::Chain& chain = wallet.chain();
6367
int confirms = wtx.GetDepthInMainChain();
6468
bool fLocked = chain.isInstantSendLockedTx(wtx.GetHash());
6569
bool chainlock = false;
6670
if (confirms > 0) {
67-
chainlock = wtx.IsChainLocked();
71+
chainlock = wallet.IsTxChainLocked(wtx);
6872
}
6973
entry.pushKV("confirmations", confirms);
7074
entry.pushKV("instantlock", fLocked || chainlock);
@@ -586,7 +590,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b
586590
|| !wallet.chain().checkFinalTx(*wtx.tx)) {
587591
continue;
588592
}
589-
if (depth < min_depth && !(fAddLocked && wtx.IsLockedByInstantSend())) continue;
593+
if (depth < min_depth && !(fAddLocked && wallet.IsTxLockedByInstantSend(wtx))) continue;
590594

591595
for (const CTxOut& txout : wtx.tx->vout) {
592596
CTxDestination address;
@@ -993,7 +997,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
993997
const CWalletTx& wtx = pairWtx.second;
994998

995999
int nDepth = wtx.GetDepthInMainChain();
996-
if ((nDepth < nMinDepth) && !(fAddLocked && wtx.IsLockedByInstantSend()))
1000+
if ((nDepth < nMinDepth) && !(fAddLocked && wallet.IsTxLockedByInstantSend(wtx)))
9971001
continue;
9981002

9991003
// Coinbase with less than 1 confirmation is no longer in the main chain
@@ -1245,14 +1249,14 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM
12451249
entry.pushKV("vout", s.vout);
12461250
entry.pushKV("fee", ValueFromAmount(-nFee));
12471251
if (fLong)
1248-
WalletTxToJSON(wallet.chain(), wtx, entry);
1252+
WalletTxToJSON(wallet, wtx, entry);
12491253
entry.pushKV("abandoned", wtx.isAbandoned());
12501254
ret.push_back(entry);
12511255
}
12521256
}
12531257

12541258
// Received
1255-
if (listReceived.size() > 0 && ((wtx.GetDepthInMainChain() >= nMinDepth) || wtx.IsLockedByInstantSend()))
1259+
if (listReceived.size() > 0 && ((wtx.GetDepthInMainChain() >= nMinDepth) || wallet.IsTxLockedByInstantSend(wtx)))
12561260
{
12571261
for (const COutputEntry& r : listReceived)
12581262
{
@@ -1292,7 +1296,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM
12921296
}
12931297
entry.pushKV("vout", r.vout);
12941298
if (fLong)
1295-
WalletTxToJSON(wallet.chain(), wtx, entry);
1299+
WalletTxToJSON(wallet, wtx, entry);
12961300
ret.push_back(entry);
12971301
}
12981302
}
@@ -1664,7 +1668,7 @@ static RPCHelpMan gettransaction()
16641668
if (wtx.IsFromMe(filter))
16651669
entry.pushKV("fee", ValueFromAmount(nFee));
16661670

1667-
WalletTxToJSON(pwallet->chain(), wtx, entry);
1671+
WalletTxToJSON(*pwallet, wtx, entry);
16681672

16691673
UniValue details(UniValue::VARR);
16701674
ListTransactions(*pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */);

‎src/wallet/transaction.h‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ class CWalletTx
7777
*/
7878
static constexpr const uint256& ABANDON_HASH = uint256::ONE;
7979

80-
mutable bool fIsChainlocked{false};
81-
mutable bool fIsInstantSendLocked{false};
82-
8380
public:
8481
/**
8582
* Key/value map with information about the transaction.
@@ -142,6 +139,8 @@ class CWalletTx
142139
mutable bool m_is_cache_empty{true};
143140
mutable bool fChangeCached;
144141
mutable bool fInMempool;
142+
mutable bool fIsChainlocked;
143+
mutable bool fIsInstantSendLocked;
145144
mutable CAmount nChangeCached;
146145

147146
CWalletTx(const CWallet* wallet, CTransactionRef arg)
@@ -161,6 +160,8 @@ class CWalletTx
161160
fFromMe = false;
162161
fChangeCached = false;
163162
fInMempool = false;
163+
fIsChainlocked = false;
164+
fIsInstantSendLocked = false;
164165
nChangeCached = 0;
165166
nOrderPos = -1;
166167
m_confirm = Confirmation{};
@@ -307,10 +308,22 @@ class CWalletTx
307308

308309
int64_t GetTxTime() const;
309310

310-
bool CanBeResent() const;
311+
// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
312+
// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
313+
// "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to
314+
// resolve the issue of member access into incomplete type CWallet. Note
315+
// that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)"
316+
// in place.
317+
bool CanBeResent() const NO_THREAD_SAFETY_ANALYSIS;
311318

312319
/** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */
313-
bool SubmitMemoryPoolAndRelay(bilingual_str& err_string, bool relay);
320+
// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
321+
// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
322+
// "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to
323+
// resolve the issue of member access into incomplete type CWallet. Note
324+
// that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)"
325+
// in place.
326+
bool SubmitMemoryPoolAndRelay(bilingual_str& err_string, bool relay) NO_THREAD_SAFETY_ANALYSIS;
314327

315328
// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
316329
// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
@@ -334,8 +347,6 @@ class CWalletTx
334347
// in place.
335348
int GetDepthInMainChain() const NO_THREAD_SAFETY_ANALYSIS;
336349
bool IsInMainChain() const { return GetDepthInMainChain() > 0; }
337-
bool IsLockedByInstantSend() const;
338-
bool IsChainLocked() const NO_THREAD_SAFETY_ANALYSIS;
339350

340351
/**
341352
* @return number of blocks to maturity for this transaction:

‎src/wallet/wallet.cpp‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11301130
auto it = mapWallet.find(hashTx);
11311131
assert(it != mapWallet.end());
11321132
const CWalletTx& origtx = it->second;
1133-
if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool() || origtx.IsLockedByInstantSend()) {
1133+
if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool() || IsTxLockedByInstantSend(origtx)) {
11341134
return false;
11351135
}
11361136

@@ -1788,7 +1788,7 @@ void CWallet::ReacceptWalletTransactions()
17881788

17891789
int nDepth = wtx.GetDepthInMainChain();
17901790

1791-
if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.IsLockedByInstantSend() && !wtx.isAbandoned())) {
1791+
if (!wtx.IsCoinBase() && (nDepth == 0 && !IsTxLockedByInstantSend(wtx) && !wtx.isAbandoned())) {
17921792
mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx));
17931793
}
17941794
}
@@ -1803,6 +1803,7 @@ void CWallet::ReacceptWalletTransactions()
18031803

18041804
bool CWalletTx::CanBeResent() const
18051805
{
1806+
AssertLockHeld(pwallet->cs_wallet);
18061807
return
18071808
// Can't relay if wallet is not broadcasting
18081809
pwallet->GetBroadcastTransactions() &&
@@ -1814,11 +1815,12 @@ bool CWalletTx::CanBeResent() const
18141815
// Don't try to submit conflicted or confirmed transactions.
18151816
GetDepthInMainChain() == 0 &&
18161817
// Don't try to submit transactions locked via InstantSend.
1817-
!IsLockedByInstantSend();
1818+
!pwallet->IsTxLockedByInstantSend(*this);
18181819
}
18191820

18201821
bool CWalletTx::SubmitMemoryPoolAndRelay(bilingual_str& err_string, bool relay)
18211822
{
1823+
AssertLockHeld(pwallet->cs_wallet);
18221824
if (!CanBeResent()) return false;
18231825

18241826
// Submit transaction to mempool for relay
@@ -3459,28 +3461,27 @@ int CWalletTx::GetDepthInMainChain() const
34593461
return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1);
34603462
}
34613463

3462-
bool CWalletTx::IsLockedByInstantSend() const
3464+
bool CWallet::IsTxLockedByInstantSend(const CWalletTx& wtx) const
34633465
{
3464-
if (fIsChainlocked) {
3465-
fIsInstantSendLocked = false;
3466-
} else if (!fIsInstantSendLocked) {
3467-
fIsInstantSendLocked = pwallet->chain().isInstantSendLockedTx(GetHash());
3466+
AssertLockHeld(cs_wallet);
3467+
if (wtx.fIsChainlocked) {
3468+
wtx.fIsInstantSendLocked = false;
3469+
} else if (!wtx.fIsInstantSendLocked) {
3470+
wtx.fIsInstantSendLocked = chain().isInstantSendLockedTx(wtx.GetHash());
34683471
}
3469-
return fIsInstantSendLocked;
3472+
return wtx.fIsInstantSendLocked;
34703473
}
34713474

3472-
bool CWalletTx::IsChainLocked() const
3475+
bool CWallet::IsTxChainLocked(const CWalletTx& wtx) const
34733476
{
3474-
if (!fIsChainlocked) {
3475-
assert(pwallet != nullptr);
3476-
AssertLockHeld(pwallet->cs_wallet);
3477-
bool active;
3478-
int height;
3479-
if (pwallet->chain().findBlock(m_confirm.hashBlock, FoundBlock().inActiveChain(active).height(height)) && active) {
3480-
fIsChainlocked = pwallet->chain().hasChainLock(height, m_confirm.hashBlock);
3477+
AssertLockHeld(cs_wallet);
3478+
if (!wtx.fIsChainlocked) {
3479+
bool active; int height;
3480+
if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().inActiveChain(active).height(height)) && active) {
3481+
wtx.fIsChainlocked = chain().hasChainLock(height, wtx.m_confirm.hashBlock);
34813482
}
34823483
}
3483-
return fIsChainlocked;
3484+
return wtx.fIsChainlocked;
34843485
}
34853486

34863487
int CWalletTx::GetBlocksToMaturity() const

‎src/wallet/wallet.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
509509
const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
510510
bool IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
511511

512+
bool IsTxLockedByInstantSend(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
513+
bool IsTxChainLocked(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
514+
512515
//! check whether we support the named feature
513516
bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); }
514517

0 commit comments

Comments
 (0)