diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 248557919a1e..162547f1dd97 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -899,6 +899,53 @@ the upper cycle, etc. Threads and synchronization ---------------------------- +- Prefer `Mutex` type to `RecursiveMutex` one + +- Consistently use [Clang Thread Safety Analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) annotations to + get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with + run-time asserts in function definitions: + +```C++ +// txmempool.h +class CTxMemPool +{ +public: + ... + mutable RecursiveMutex cs; + ... + void UpdateTransactionsFromBlock(...) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, cs); + ... +} + +// txmempool.cpp +void CTxMemPool::UpdateTransactionsFromBlock(...) +{ + AssertLockHeld(::cs_main); + AssertLockHeld(cs); + ... +} +``` + +```C++ +// validation.h +class ChainstateManager +{ +public: + ... + bool ProcessNewBlock(...) LOCKS_EXCLUDED(::cs_main); + ... +} + +// validation.cpp +bool ChainstateManager::ProcessNewBlock(...) +{ + AssertLockNotHeld(::cs_main); + ... + LOCK(::cs_main); + ... +} +``` + - Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential deadlocks are introduced. diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 9739727243ed..4c15f98f4a51 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -1448,11 +1448,16 @@ bool CCoinJoinClientSession::MakeCollateralAmounts() }); // First try to use only non-denominated funds - if (std::any_of(vecTally.begin(), vecTally.end(), [&](const auto& item) { return MakeCollateralAmounts(item, false); })) { + if (ranges::any_of(vecTally, [&](const auto& item) EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet) { + return MakeCollateralAmounts(item, false); + })) { return true; } + // There should be at least some denominated funds we should be able to break in pieces to continue mixing - if (std::any_of(vecTally.begin(), vecTally.end(), [&](const auto& item) { return MakeCollateralAmounts(item, true); })) { + if (ranges::any_of(vecTally, [&](const auto& item) EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet) { + return MakeCollateralAmounts(item, true); + })) { return true; } diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index a0deebdbd24b..c4e22b71230c 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -161,13 +161,16 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession /// Create denominations bool CreateDenominated(CAmount nBalanceToDenominate); - bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals); + bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet); /// Split up large inputs or make fee sized inputs bool MakeCollateralAmounts(); - bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated); + bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet); - bool CreateCollateralTransaction(CMutableTransaction& txCollateral, std::string& strReason); + bool CreateCollateralTransaction(CMutableTransaction& txCollateral, std::string& strReason) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet); bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman); bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman); @@ -175,7 +178,9 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession /// step 0: select denominated inputs and txouts bool SelectDenominate(std::string& strErrorRet, std::vector& vecTxDSInRet); /// step 1: prepare denominated inputs and outputs - bool PrepareDenominate(int nMinRounds, int nMaxRounds, std::string& strErrorRet, const std::vector& vecTxDSIn, std::vector >& vecPSInOutPairsRet, bool fDryRun = false); + bool PrepareDenominate(int nMinRounds, int nMaxRounds, std::string& strErrorRet, const std::vector& vecTxDSIn, + std::vector>& vecPSInOutPairsRet, bool fDryRun = false) + EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet); /// step 2: send denominated inputs and outputs prepared in step 1 bool SendDenominate(const std::vector >& vecPSInOutPairsIn, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 6bf8e93e112b..5680b8abc20f 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -398,8 +398,8 @@ class CDSTXManager private: void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx); - void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight); - + void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional nHeight) + EXCLUSIVE_LOCKS_REQUIRED(cs_mapdstx); }; bool ATMPIfSaneFee(ChainstateManager& chainman, const CTransactionRef& tx, bool test_accept = false) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index ac167a298bb3..49912b888061 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -1067,11 +1067,13 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n mnListsCache.emplace(snapshot.GetBlockHash(), snapshot); } else { // keep snapshots for yet alive quorums - if (ranges::any_of(Params().GetConsensus().llmqs, [&snapshot, this](const auto& params){ - AssertLockHeld(cs); - return (snapshot.GetHeight() % params.dkgInterval == 0) && - (snapshot.GetHeight() + params.dkgInterval * (params.keepOldConnections + 1) >= tipIndex->nHeight); - })) { + if (ranges::any_of(Params().GetConsensus().llmqs, + [&snapshot, this](const auto& params) EXCLUSIVE_LOCKS_REQUIRED(cs) { + AssertLockHeld(cs); + return (snapshot.GetHeight() % params.dkgInterval == 0) && + (snapshot.GetHeight() + params.dkgInterval * (params.keepOldConnections + 1) >= + tipIndex->nHeight); + })) { mnListsCache.emplace(snapshot.GetBlockHash(), snapshot); } } diff --git a/src/evo/simplifiedmns.h b/src/evo/simplifiedmns.h index 9999a5f35205..0cfd5603d9bb 100644 --- a/src/evo/simplifiedmns.h +++ b/src/evo/simplifiedmns.h @@ -171,8 +171,9 @@ class CSimplifiedMNListDiff [[nodiscard]] UniValue ToJson(bool extended = false) const; }; -bool BuildSimplifiedMNListDiff(CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const llmq::CQuorumBlockProcessor& qblockman, - const llmq::CQuorumManager& qman, const uint256& baseBlockHash, const uint256& blockHash, - CSimplifiedMNListDiff& mnListDiffRet, std::string& errorRet, bool extended = false); +bool BuildSimplifiedMNListDiff(CDeterministicMNManager& dmnman, const ChainstateManager& chainman, + const llmq::CQuorumBlockProcessor& qblockman, const llmq::CQuorumManager& qman, + const uint256& baseBlockHash, const uint256& blockHash, CSimplifiedMNListDiff& mnListDiffRet, + std::string& errorRet, bool extended = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); #endif // BITCOIN_EVO_SIMPLIFIEDMNS_H diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index e20a9e0d4f7b..c8c84e84bed4 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -19,9 +19,10 @@ #include #include -static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const llmq::CQuorumManager& qman, const CTransaction& tx, - const CBlockIndex* pindexPrev, const CCoinsViewCache& view, const std::optional& indexes, bool check_sigs, - TxValidationState& state) +static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, const ChainstateManager& chainman, + const llmq::CQuorumManager& qman, const CTransaction& tx, const CBlockIndex* pindexPrev, + const CCoinsViewCache& view, const std::optional& indexes, bool check_sigs, + TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(cs_main); diff --git a/src/governance/governance.h b/src/governance/governance.h index e5b8e077a8ee..adc25bb5431c 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -347,9 +347,9 @@ class CGovernanceManager : public GovernanceStore * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - std::vector> GetActiveTriggers() const; - bool AddNewTrigger(uint256 nHash); - void CleanAndRemoveTriggers(); + std::vector> GetActiveTriggers() const EXCLUSIVE_LOCKS_REQUIRED(cs); + bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); + void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); // Superblocks related: @@ -373,7 +373,8 @@ class CGovernanceManager : public GovernanceStore private: void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); - bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight); + bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(cs); std::optional CreateSuperblockCandidate(int nHeight) const; std::optional CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, diff --git a/src/governance/object.h b/src/governance/object.h index e58da6a8a1be..8ced4950f939 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -235,7 +235,8 @@ class CGovernanceObject /// Check the collateral transaction for the budget proposal/finalized budget bool IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman); + void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); void UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list); diff --git a/src/index/base.h b/src/index/base.h index 403c8c87b1b2..e1f2a9b82d9d 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -118,7 +118,7 @@ class BaseIndex : public CValidationInterface /// sync once and only needs to process blocks in the ValidationInterface /// queue. If the index is catching up from far behind, this method does /// not block and immediately returns false. - bool BlockUntilSyncedToCurrentChain() const; + bool BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(::cs_main); void Interrupt(); diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 0b157f218ab5..47a70430fa68 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -565,24 +565,29 @@ void CSigSharesManager::CollectPendingSigSharesToVerify( // other nodes would be able to poison us with a large batch with N-1 valid shares and the last one being // invalid, making batch verification fail and revert to per-share verification, which in turn would slow down // the whole verification process - std::unordered_set, StaticSaltedHasher> uniqueSignHashes; - IterateNodesRandom(nodeStates, [&]() { - return uniqueSignHashes.size() < maxUniqueSessions; - }, [&](NodeId nodeId, CSigSharesNodeState& ns) { - if (ns.pendingIncomingSigShares.Empty()) { - return false; - } - const auto& sigShare = *ns.pendingIncomingSigShares.GetFirst(); + IterateNodesRandom( + nodeStates, + [&]() { + return uniqueSignHashes.size() < maxUniqueSessions; + // TODO: remove NO_THREAD_SAFETY_ANALYSIS + // using here template IterateNodesRandom makes impossible to use lock annotation + }, + [&](NodeId nodeId, CSigSharesNodeState& ns) NO_THREAD_SAFETY_ANALYSIS { + if (ns.pendingIncomingSigShares.Empty()) { + return false; + } + const auto& sigShare = *ns.pendingIncomingSigShares.GetFirst(); - AssertLockHeld(cs); - if (const bool alreadyHave = this->sigShares.Has(sigShare.GetKey()); !alreadyHave) { - uniqueSignHashes.emplace(nodeId, sigShare.GetSignHash()); - retSigShares[nodeId].emplace_back(sigShare); - } - ns.pendingIncomingSigShares.Erase(sigShare.GetKey()); - return !ns.pendingIncomingSigShares.Empty(); - }, rnd); + AssertLockHeld(cs); + if (const bool alreadyHave = this->sigShares.Has(sigShare.GetKey()); !alreadyHave) { + uniqueSignHashes.emplace(nodeId, sigShare.GetSignHash()); + retSigShares[nodeId].emplace_back(sigShare); + } + ns.pendingIncomingSigShares.Erase(sigShare.GetKey()); + return !ns.pendingIncomingSigShares.Empty(); + }, + rnd); if (retSigShares.empty()) { return; @@ -1020,7 +1025,10 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map, std::unordered_set, StaticSaltedHasher> quorumNodesMap; - sigSharesQueuedToAnnounce.ForEach([this, &quorumNodesMap, &sigSharesToAnnounce](const SigShareKey& sigShareKey, bool) { + // TODO: remove NO_THREAD_SAFETY_ANALYSIS + // using here template ForEach makes impossible to use lock annotation + sigSharesQueuedToAnnounce.ForEach([this, &quorumNodesMap, &sigSharesToAnnounce](const SigShareKey& sigShareKey, + bool) NO_THREAD_SAFETY_ANALYSIS { AssertLockHeld(cs); const auto& signHash = sigShareKey.first; auto quorumMember = sigShareKey.second; @@ -1076,7 +1084,7 @@ bool CSigSharesManager::SendMessages() std::unordered_map> sigSharesToAnnounce; std::unordered_map> sigSessionAnnouncements; - auto addSigSesAnnIfNeeded = [&](NodeId nodeId, const uint256& signHash) { + auto addSigSesAnnIfNeeded = [&](NodeId nodeId, const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs) { AssertLockHeld(cs); auto& nodeState = nodeStates[nodeId]; auto* session = nodeState.GetSessionBySignHash(signHash); @@ -1357,7 +1365,9 @@ void CSigSharesManager::Cleanup() continue; } // remove global requested state to force a re-request from another node - it->second.requestedSigShares.ForEach([this](const SigShareKey& k, bool) { + // TODO: remove NO_THREAD_SAFETY_ANALYSIS + // using here template ForEach makes impossible to use lock annotation + it->second.requestedSigShares.ForEach([this](const SigShareKey& k, bool) NO_THREAD_SAFETY_ANALYSIS { AssertLockHeld(cs); sigSharesRequested.Erase(k); }); @@ -1390,7 +1400,9 @@ void CSigSharesManager::RemoveBannedNodeStates() for (auto it = nodeStates.begin(); it != nodeStates.end();) { if (Assert(m_peerman)->IsBanned(it->first)) { // re-request sigshares from other nodes - it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) { + // TODO: remove NO_THREAD_SAFETY_ANALYSIS + // using here template ForEach makes impossible to use lock annotation + it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS { AssertLockHeld(cs); sigSharesRequested.Erase(k); }); @@ -1419,7 +1431,9 @@ void CSigSharesManager::BanNode(NodeId nodeId) auto& nodeState = it->second; // Whatever we requested from him, let's request it from someone else now - nodeState.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) { + // TODO: remove NO_THREAD_SAFETY_ANALYSIS + // using here template ForEach makes impossible to use lock annotation + nodeState.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS { AssertLockHeld(cs); sigSharesRequested.Erase(k); }); diff --git a/src/llmq/snapshot.h b/src/llmq/snapshot.h index b9ca9ac985dd..1551789f3779 100644 --- a/src/llmq/snapshot.h +++ b/src/llmq/snapshot.h @@ -209,7 +209,7 @@ class CQuorumRotationInfo bool BuildQuorumRotationInfo(CDeterministicMNManager& dmnman, const ChainstateManager& chainman, const CQuorumManager& qman, const CQuorumBlockProcessor& qblockman, const CGetQuorumRotationInfo& request, - CQuorumRotationInfo& response, std::string& errorRet); + CQuorumRotationInfo& response, std::string& errorRet) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); uint256 GetLastBaseBlockHash(Span baseBlockIndexes, const CBlockIndex* blockIndex); class CQuorumSnapshotManager diff --git a/src/masternode/meta.h b/src/masternode/meta.h index 0388905c4462..14870afa636c 100644 --- a/src/masternode/meta.h +++ b/src/masternode/meta.h @@ -73,7 +73,11 @@ class CMasternodeMetaInfo UniValue ToJson() const; public: - const uint256& GetProTxHash() const { LOCK(cs); return proTxHash; } + const uint256 GetProTxHash() const + { + LOCK(cs); + return proTxHash; + } int64_t GetLastDsq() const { return nLastDsq; } int GetMixingTxCount() const { return nMixingTxCount; } diff --git a/src/net.cpp b/src/net.cpp index 474fbec3e701..2b6fd4608bb4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -643,8 +643,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo void CNode::CloseSocketDisconnect(CConnman* connman) { - AssertLockHeld(connman->m_nodes_mutex); - fDisconnect = true; LOCK2(connman->cs_mapSocketToNode, m_sock_mutex); if (!m_sock) { @@ -3512,8 +3510,8 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, MasternodeProbeConn isProbe = MasternodeProbeConn::IsNotConnection; - const auto getPendingQuorumNodes = [&]() { - LockAssertion lock(cs_vPendingMasternodes); + const auto getPendingQuorumNodes = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_vPendingMasternodes) { + AssertLockHeld(cs_vPendingMasternodes); std::vector ret; for (const auto& group : masternodeQuorumNodes) { for (const auto& proRegTxHash : group.second) { @@ -3551,8 +3549,8 @@ void CConnman::ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, return ret; }; - const auto getPendingProbes = [&]() { - LockAssertion lock(cs_vPendingMasternodes); + const auto getPendingProbes = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_vPendingMasternodes) { + AssertLockHeld(cs_vPendingMasternodes); std::vector ret; for (auto it = masternodePendingProbes.begin(); it != masternodePendingProbes.end(); ) { auto dmn = mnList.GetMN(*it); diff --git a/src/net.h b/src/net.h index 248a8a6b0401..c34c87104a6c 100644 --- a/src/net.h +++ b/src/net.h @@ -1275,6 +1275,8 @@ friend class CNode; return ForNode(id, FullyConnectedOnly, func); } + using NodeFn = std::function; + bool IsConnected(const CService& addr, std::function cond) { return ForNode(addr, cond, [](CNode* pnode){ @@ -1331,10 +1333,9 @@ friend class CNode; } }; - template - void ForEachNode(Callable&& func) + void ForEachNode(const NodeFn& fn) { - ForEachNode(FullyConnectedOnly, func); + ForEachNode(FullyConnectedOnly, fn); } template @@ -1347,10 +1348,9 @@ friend class CNode; } }; - template - void ForEachNode(Callable&& func) const + void ForEachNode(const NodeFn& fn) const { - ForEachNode(FullyConnectedOnly, func); + ForEachNode(FullyConnectedOnly, fn); } template diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9536b8fd07b6..d05eb8917e1c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -944,7 +944,7 @@ class PeerManagerImpl final : public PeerManager /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ CTransactionRef FindTxForGetData(const CNode* peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main); - void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) LOCKS_EXCLUDED(cs_main) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex); + void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex) LOCKS_EXCLUDED(::cs_main); /** Process a new block. Perform any post-processing housekeeping */ void ProcessBlock(CNode& from, const std::shared_ptr& pblock, bool force_processing); @@ -1930,12 +1930,14 @@ void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler) */ void PeerManagerImpl::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex) { - LOCK2(::cs_main, g_cs_orphans); + { + LOCK2(::cs_main, g_cs_orphans); - auto orphanWorkSet = m_orphanage.GetCandidatesForBlock(*pblock); - while (!orphanWorkSet.empty()) { - LogPrint(BCLog::MEMPOOL, "Trying to process %d orphans\n", orphanWorkSet.size()); - ProcessOrphanTx(orphanWorkSet); + auto orphanWorkSet = m_orphanage.GetCandidatesForBlock(*pblock); + while (!orphanWorkSet.empty()) { + LogPrint(BCLog::MEMPOOL, "Trying to process %d orphans\n", orphanWorkSet.size()); + ProcessOrphanTx(orphanWorkSet); + } } m_orphanage.EraseForBlock(*pblock); @@ -1998,8 +2000,8 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha m_most_recent_compact_block = pcmpctblock; } - m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) { - LockAssertion lock(::cs_main); + m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); // TODO: Avoid the repeated-serialization here if (pnode->fDisconnect) return; @@ -5273,8 +5275,8 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) // We want to prevent recently connected to Onion peers from being disconnected here, protect them as long as // there are more non_onion nodes than onion nodes so far size_t onion_count = 0; - m_connman.ForEachNode([&](CNode* pnode) { - LockAssertion lock(::cs_main); + m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); if (pnode->addr.IsTor() && ++onion_count <= m_connman.GetMaxOutboundOnionNodeCount()) return; // Don't disconnect masternodes just because they were slow in block announcement if (pnode->m_masternode_connection) return; @@ -5291,8 +5293,8 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) } }); if (worst_peer != -1) { - bool disconnected = m_connman.ForNode(worst_peer, [&](CNode *pnode) { - LockAssertion lock(::cs_main); + bool disconnected = m_connman.ForNode(worst_peer, [&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); // Only disconnect a peer that has been connected to us for // some reasonable fraction of our check-frequency, to give @@ -5771,7 +5773,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) peer->m_blocks_for_inv_relay.clear(); } - auto queueAndMaybePushInv = [this, pto, peer, &vInv, &msgMaker](const CInv& invIn) { + auto queueAndMaybePushInv = [this, pto, peer, &vInv, &msgMaker](const CInv& invIn) EXCLUSIVE_LOCKS_REQUIRED(peer->m_tx_relay->m_tx_inventory_mutex) { AssertLockHeld(peer->m_tx_relay->m_tx_inventory_mutex); peer->m_tx_relay->m_tx_inventory_known_filter.insert(invIn.hash); LogPrint(BCLog::NET, "SendMessages -- queued inv: %s index=%d peer=%d\n", invIn.ToString(), vInv.size(), pto->GetId()); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 30fdab04b734..5fa7506cdaa3 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -213,8 +213,9 @@ static bool ValidatePlatformPort(const int32_t port) #ifdef ENABLE_WALLET -template -static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const SpecialTxPayload& payload, const CTxDestination& fundDest) +template +static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const SpecialTxPayload& payload, + const CTxDestination& fundDest) EXCLUSIVE_LOCKS_REQUIRED(!wallet.cs_wallet) { // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now diff --git a/src/sync.cpp b/src/sync.cpp index 35d3ffaf120d..754081926253 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -285,12 +285,16 @@ template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); template void AssertLockHeldInternal(const char*, const char*, int, SharedMutex*); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +template +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { if (!LockHeld(cs)) return; tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockNotHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockNotHeldInternal(const char*, const char*, int, RecursiveMutex*); +template void AssertLockNotHeldInternal(const char*, const char*, int, SharedMutex*); void DeleteLock(void* cs) { diff --git a/src/sync.h b/src/sync.h index 3712bd9824e5..078ee4f6e8a5 100644 --- a/src/sync.h +++ b/src/sync.h @@ -57,8 +57,9 @@ void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); template -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); +template +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs); void DeleteLock(void* cs); bool LockStackEmpty(); @@ -74,8 +75,9 @@ inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, M inline void LeaveCritical() {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template -inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -inline void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {} +template +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) LOCKS_EXCLUDED(cs) {} inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif @@ -450,18 +452,4 @@ class CSemaphoreGrant } }; -// Utility class for indicating to compiler thread analysis that a mutex is -// locked (when it couldn't be determined otherwise). -struct SCOPED_LOCKABLE LockAssertion -{ - template - explicit LockAssertion(Mutex& mutex) EXCLUSIVE_LOCK_FUNCTION(mutex) - { -#ifdef DEBUG_LOCKORDER - AssertLockHeld(mutex); -#endif - } - ~LockAssertion() UNLOCK_FUNCTION() {} -}; - #endif // BITCOIN_SYNC_H diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cb16e2ddfc7b..291599aea201 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -955,7 +955,7 @@ void CTxMemPool::removeProTxSpentCollateralConflicts(const CTransaction &tx) assert(m_dmnman); // Remove TXs that refer to a MN for which the collateral was spent - auto removeSpentCollateralConflict = [&](const uint256& proTxHash) { + auto removeSpentCollateralConflict = [&](const uint256& proTxHash) EXCLUSIVE_LOCKS_REQUIRED(cs) { // Can't use equal_range here as every call to removeRecursive might invalidate iterators AssertLockHeld(cs); while (true) { @@ -1353,7 +1353,7 @@ bool CTxMemPool::existsProviderTxConflict(const CTransaction &tx) const { LOCK(cs); - auto hasKeyChangeInMempool = [&](const uint256& proTxHash) { + auto hasKeyChangeInMempool = [&](const uint256& proTxHash) EXCLUSIVE_LOCKS_REQUIRED(cs) { AssertLockHeld(cs); for (auto its = mapProTxRefs.equal_range(proTxHash); its.first != its.second; ++its.first) { auto txit = mapTx.find(its.first->second); diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 84835e0aad61..ce9d6435ce9f 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -197,7 +197,7 @@ std::set TxOrphanage::GetCandidatesForBlock(const CBlock& block) void TxOrphanage::EraseForBlock(const CBlock& block) { - AssertLockHeld(g_cs_orphans); + LOCK(g_cs_orphans); std::vector vOrphanErase; diff --git a/src/txorphanage.h b/src/txorphanage.h index 6fc043eda607..d8ef7a5c44b9 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -41,7 +41,7 @@ class TxOrphanage { void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); /** Erase all orphans included in or invalidated by a new block */ - void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); + void EraseForBlock(const CBlock& block) LOCKS_EXCLUDED(::g_cs_orphans); /** Limit the orphanage to the given maximum */ unsigned int LimitOrphans(unsigned int max_orphans_size) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans); diff --git a/src/util/system.cpp b/src/util/system.cpp index b0af7eb30b6b..9ecadeabce70 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -420,7 +420,7 @@ std::optional ArgsManager::GetArgFlags(const std::string& name) co return std::nullopt; } -const fs::path& ArgsManager::GetBlocksDirPath() const +const fs::path ArgsManager::GetBlocksDirPath() const { LOCK(cs_args); fs::path& path = m_cached_blocks_path; @@ -445,7 +445,7 @@ const fs::path& ArgsManager::GetBlocksDirPath() const return path; } -const fs::path& ArgsManager::GetDataDir(bool net_specific) const +const fs::path ArgsManager::GetDataDir(bool net_specific) const { LOCK(cs_args); fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path; diff --git a/src/util/system.h b/src/util/system.h index 1ae230d4f80e..4ab19054596a 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -288,7 +288,7 @@ class ArgsManager * * @return Blocks path which is network specific */ - const fs::path& GetBlocksDirPath() const; + const fs::path GetBlocksDirPath() const; /** * Get data directory path @@ -296,7 +296,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path& GetDataDirBase() const { return GetDataDir(false); } + const fs::path GetDataDirBase() const { return GetDataDir(false); } /** * Get data directory path with appended network identifier @@ -304,7 +304,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path& GetDataDirNet() const { return GetDataDir(true); } + const fs::path GetDataDirNet() const { return GetDataDir(true); } fs::path GetBackupsDirPath(); @@ -483,7 +483,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path& GetDataDir(bool net_specific) const; + const fs::path GetDataDir(bool net_specific) const; // Helper function for LogArgs(). void logArgsPrefix( diff --git a/src/validation.cpp b/src/validation.cpp index 38a030ace1c6..afd909c993d3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3644,9 +3644,10 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) +static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - AssertLockHeld(cs_main); + // TODO: validate - why do we need this cs_main ? + AssertLockHeld(::cs_main); const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; // Enforce BIP113 (Median Time Past). diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 9d84a04b6a32..52d07ff7fa50 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -342,8 +342,11 @@ void LegacyScriptPubKeyMan::UpgradeKeyMetadata() CHDChain hdChainCurrent; if (!GetHDChain(hdChainCurrent)) throw std::runtime_error(std::string(__func__) + ": GetHDChain failed"); - if (!DecryptHDChain(m_storage.GetEncryptionKey(), hdChainCurrent)) + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptHDChain(encryption_key, hdChainCurrent); + })) { throw std::runtime_error(std::string(__func__) + ": DecryptHDChain failed"); + } CExtKey masterKey; SecureVector vchSeed = hdChainCurrent.GetSeed(); @@ -474,8 +477,11 @@ bool LegacyScriptPubKeyMan::GetDecryptedHDChain(CHDChain& hdChainRet) return false; } - if (!DecryptHDChain(m_storage.GetEncryptionKey(), hdChainTmp)) + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptHDChain(encryption_key, hdChainTmp); + })) { return false; + } // make sure seed matches this chain if (hdChainTmp.GetID() != hdChainTmp.GetSeedHash()) @@ -911,7 +917,9 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu std::vector vchCryptedSecret; CKeyingMaterial vchSecret(key.begin(), key.end()); - if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, vchSecret, pubkey.GetHash(), vchCryptedSecret); + })) { return false; } @@ -933,7 +941,9 @@ bool LegacyScriptPubKeyMan::GetKeyInner(const CKeyID &address, CKey& keyOut) con { const CPubKey &vchPubKey = (*mi).second.first; const std::vector &vchCryptedSecret = (*mi).second.second; - return DecryptKey(m_storage.GetEncryptionKey(), vchCryptedSecret, vchPubKey, keyOut); + return m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, vchCryptedSecret, vchPubKey, keyOut); + }); } return false; } @@ -1164,8 +1174,11 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const CHDChain hdChainCurrent; if (!GetHDChain(hdChainCurrent)) throw std::runtime_error(std::string(__func__) + ": GetHDChain failed"); - if (!DecryptHDChain(m_storage.GetEncryptionKey(), hdChainCurrent)) + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptHDChain(encryption_key, hdChainCurrent); + })) { throw std::runtime_error(std::string(__func__) + ": DecryptHDChain failed"); + } // make sure seed matches this chain if (hdChainCurrent.GetID() != hdChainCurrent.GetSeedHash()) throw std::runtime_error(std::string(__func__) + ": Wrong HD chain!"); @@ -1276,8 +1289,11 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& throw std::runtime_error(std::string(__func__) + ": GetHDChain failed"); } - if (!DecryptHDChain(m_storage.GetEncryptionKey(), hdChainTmp)) + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptHDChain(encryption_key, hdChainTmp); + })) { throw std::runtime_error(std::string(__func__) + ": DecryptHDChain failed"); + } // make sure seed matches this chain if (hdChainTmp.GetID() != hdChainTmp.GetSeedHash()) throw std::runtime_error(std::string(__func__) + ": Wrong HD chain!"); @@ -1897,7 +1913,9 @@ std::map DescriptorScriptPubKeyMan::GetKeys() const const CPubKey& pubkey = key_pair.second.first; const std::vector& crypted_secret = key_pair.second.second; CKey key; - DecryptKey(m_storage.GetEncryptionKey(), crypted_secret, pubkey, key); + m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, crypted_secret, pubkey, key); + }); keys[pubkey.GetID()] = key; } return keys; @@ -2010,7 +2028,9 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const std::vector crypted_secret; CKeyingMaterial secret(key.begin(), key.end()); - if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, secret, pubkey.GetHash(), crypted_secret); + })) { return false; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 0d6450d671a7..e3f1916272c8 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -21,6 +21,7 @@ #include +#include #include // Wallet storage things that ScriptPubKeyMans need in order to be able to store things to the wallet database. @@ -38,7 +39,8 @@ class WalletStorage virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; virtual bool CanSupportFeature(enum WalletFeature) const = 0; virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr) = 0; - virtual const CKeyingMaterial& GetEncryptionKey() const = 0; + //! Pass the encryption key to cb(). + virtual bool WithEncryptionKey(std::function cb) const = 0; virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked(bool fForMixing) const = 0; @@ -526,7 +528,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan //! keeps track of whether Unlock has run a thorough check before bool m_decryption_thoroughly_checked = false; - bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey); + bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index edc7b41c9ef8..00d872582a5f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3681,7 +3681,7 @@ bool CWallet::CreateTransactionInternal( txNew.vin.emplace_back(coin.outpoint, CScript(), CTxIn::SEQUENCE_FINAL - 1); } - auto calculateFee = [&](CAmount& nFee) -> bool { + auto calculateFee = [&](CAmount& nFee) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) -> bool { AssertLockHeld(cs_wallet); nBytes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly); if (nBytes < 0) { @@ -5743,9 +5743,10 @@ void CWallet::SetupLegacyScriptPubKeyMan() m_spk_managers[spk_manager->GetID()] = std::move(spk_manager); } -const CKeyingMaterial& CWallet::GetEncryptionKey() const +bool CWallet::WithEncryptionKey(std::function cb) const { - return vMasterKey; + LOCK(cs_wallet); + return cb(vMasterKey); } bool CWallet::HasEncryptionKeys() const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 11824d79ee5e..af80efb535bc 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -516,8 +516,12 @@ class CWalletTx CAmount GetImmatureWatchOnlyCredit(const bool fUseCache = true) const; CAmount GetChange() const; - CAmount GetAnonymizedCredit(const CCoinControl* coinControl = nullptr) const; - CAmount GetDenominatedCredit(bool unconfirmed, bool fUseCache=true) const; + // 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 GetAnonymizedCredit(const CCoinControl* coinControl = nullptr) const NO_THREAD_SAFETY_ANALYSIS; + CAmount GetDenominatedCredit(bool unconfirmed, bool fUseCache=true) const NO_THREAD_SAFETY_ANALYSIS; /** Get the marginal bytes if spending the specified output from this transaction */ int GetSpendSize(unsigned int out, bool use_max_sig = false) const @@ -569,7 +573,7 @@ class CWalletTx int GetDepthInMainChain() const NO_THREAD_SAFETY_ANALYSIS; bool IsInMainChain() const { return GetDepthInMainChain() > 0; } bool IsLockedByInstantSend() const; - bool IsChainLocked() const; + bool IsChainLocked() const NO_THREAD_SAFETY_ANALYSIS; /** * @return number of blocks to maturity for this transaction: @@ -1289,7 +1293,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::set GetConflicts(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Check if a given transaction has any of its outputs spent by another transaction in the wallet - bool HasWalletSpend(const uint256& txid) const; + bool HasWalletSpend(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Flush wallet (bitdb flush) void Flush(); @@ -1386,11 +1390,11 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void notifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr& clsig) override; /** Load a CGovernanceObject into m_gobjects. */ - bool LoadGovernanceObject(const Governance::Object& obj); + bool LoadGovernanceObject(const Governance::Object& obj) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Store a CGovernanceObject in the wallet database. This should only be used by governance objects that are created by this wallet via `gobject prepare`. */ - bool WriteGovernanceObject(const Governance::Object& obj); + bool WriteGovernanceObject(const Governance::Object& obj) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Returns a vector containing pointers to the governance objects in m_gobjects */ - std::vector GetGovernanceObjects(); + std::vector GetGovernanceObjects() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Blocks until the wallet state is up-to-date to /at least/ the current @@ -1398,7 +1402,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * Obviously holding cs_main/cs_wallet when going into this call may cause * deadlock */ - void BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(cs_main, cs_wallet); + void BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(::cs_main) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet); /** set a single wallet flag */ void SetWalletFlag(uint64_t flags); @@ -1464,7 +1468,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Make a LegacyScriptPubKeyMan and set it for all types, internal, and external. void SetupLegacyScriptPubKeyMan(); - const CKeyingMaterial& GetEncryptionKey() const override; + bool WithEncryptionKey(std::function cb) const override; + bool HasEncryptionKeys() const override; /** Get last block processed height */ @@ -1510,7 +1515,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void DeactivateScriptPubKeyMan(uint256 id, bool internal); //! Create new DescriptorScriptPubKeyMans and add them to the wallet - void SetupDescriptorScriptPubKeyMans(); + void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const;