Skip to content

Commit a55db4e

Browse files
committed
Add more proper thread safety annotations
1 parent 8cfe93e commit a55db4e

File tree

5 files changed

+33
-15
lines changed

5 files changed

+33
-15
lines changed

src/wallet/interfaces.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
8080

8181
//! Construct wallet tx status struct.
8282
WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx)
83+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
8384
{
85+
AssertLockHeld(wallet.cs_wallet);
86+
8487
WalletTxStatus result;
8588
result.block_height =
8689
wtx.state<TxStateConfirmed>() ? wtx.state<TxStateConfirmed>()->confirmed_block_height :

src/wallet/receive.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CW
123123

124124
CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
125125
{
126+
AssertLockHeld(wallet.cs_wallet);
127+
126128
// Must wait until coinbase is safely deep enough in the chain before valuing it
127129
if (wallet.IsTxImmatureCoinBase(wtx))
128130
return 0;
@@ -164,6 +166,8 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx)
164166

165167
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache)
166168
{
169+
AssertLockHeld(wallet.cs_wallet);
170+
167171
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
168172
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache);
169173
}
@@ -173,6 +177,8 @@ CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, b
173177

174178
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache)
175179
{
180+
AssertLockHeld(wallet.cs_wallet);
181+
176182
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
177183
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache);
178184
}

src/wallet/receive.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,15 @@ bool OutputIsChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_
2424
CAmount OutputGetChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
2525
CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx);
2626

27-
CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
27+
CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
28+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
2829
//! filter decides which addresses will count towards the debit
2930
CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
3031
CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx);
31-
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true);
32-
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true);
32+
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true)
33+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
34+
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true)
35+
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
3336
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE)
3437
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
3538
struct COutputEntry

src/wallet/wallet.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1841,6 +1841,8 @@ void CWallet::ReacceptWalletTransactions()
18411841

18421842
bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
18431843
{
1844+
AssertLockHeld(cs_wallet);
1845+
18441846
// Can't relay if wallet is not broadcasting
18451847
if (!GetBroadcastTransactions()) return false;
18461848
// Don't relay abandoned transactions
@@ -3126,15 +3128,20 @@ int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const
31263128

31273129
int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const
31283130
{
3129-
if (!wtx.IsCoinBase())
3131+
AssertLockHeld(cs_wallet);
3132+
3133+
if (!wtx.IsCoinBase()) {
31303134
return 0;
3135+
}
31313136
int chain_depth = GetTxDepthInMainChain(wtx);
31323137
assert(chain_depth >= 0); // coinbase tx should not be conflicted
31333138
return std::max(0, (COINBASE_MATURITY+1) - chain_depth);
31343139
}
31353140

31363141
bool CWallet::IsTxImmatureCoinBase(const CWalletTx& wtx) const
31373142
{
3143+
AssertLockHeld(cs_wallet);
3144+
31383145
// note GetBlocksToMaturity is 0 for non-coinbase tx
31393146
return GetTxBlocksToMaturity(wtx) > 0;
31403147
}

src/wallet/wallet.h

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -423,22 +423,20 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
423423
* 0 : in memory pool, waiting to be included in a block
424424
* >=1 : this many blocks deep in the main chain
425425
*/
426-
// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
427-
// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation
428-
// "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to
429-
// resolve the issue of member access into incomplete type CWallet. Note
430-
// that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)"
431-
// in place.
432-
int GetTxDepthInMainChain(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS;
433-
bool IsTxInMainChain(const CWalletTx& wtx) const { return GetTxDepthInMainChain(wtx) > 0; }
426+
int GetTxDepthInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
427+
bool IsTxInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)
428+
{
429+
AssertLockHeld(cs_wallet);
430+
return GetTxDepthInMainChain(wtx) > 0;
431+
}
434432

435433
/**
436434
* @return number of blocks to maturity for this transaction:
437435
* 0 : is not a coinbase transaction, or is a mature coinbase transaction
438436
* >0 : is a coinbase transaction which matures in this many blocks
439437
*/
440-
int GetTxBlocksToMaturity(const CWalletTx& wtx) const;
441-
bool IsTxImmatureCoinBase(const CWalletTx& wtx) const;
438+
int GetTxBlocksToMaturity(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
439+
bool IsTxImmatureCoinBase(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
442440

443441
//! check whether we support the named feature
444442
bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); }
@@ -578,7 +576,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
578576
void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm);
579577

580578
/** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */
581-
bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const;
579+
bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
580+
EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
582581

583582
bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const
584583
{

0 commit comments

Comments
 (0)