From a55db4ea1cf10e0ab4a6eb5cd1dd3bd95626fba0 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 16 May 2022 20:50:29 +0200 Subject: Add more proper thread safety annotations --- src/wallet/interfaces.cpp | 3 +++ src/wallet/receive.cpp | 6 ++++++ src/wallet/receive.h | 9 ++++++--- src/wallet/wallet.cpp | 9 ++++++++- src/wallet/wallet.h | 21 ++++++++++----------- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 98e843385c..49d7d6584b 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() ? wtx.state()->confirmed_block_height : diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index b38e09b454..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); } diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 06724ee473..1caef293f2 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -24,12 +24,15 @@ 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); +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 diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 97dcfda051..a91cc17e1b 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 @@ -3126,8 +3128,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); @@ -3135,6 +3140,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 54a0999d62..7da601c3b7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -423,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); } @@ -578,7 +576,8 @@ public: void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> 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 &txouts, const CCoinControl* coin_control = nullptr) const { -- cgit v1.2.3