Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/bls/bls_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class CBLSWorker
std::vector<SigVerifyJob> sigVerifyQueue;

public:
CBLSWorker(const CBLSWorker&) = delete;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you know that you already can't create a copy of CBLSWorker?

All classes that has a non-copyable member are uncoppyable too.
CBLSWorker have 2 of them:

  • std::mutex sigVerifyMutex;
  • ctpl::thread_pool workerPool

Same applieable for CChainLocksHandler; un-copyable members:

  • std::unique_ptr scheduler;
  • std::unique_ptrstd::thread scheduler_thread;
  • mutable Mutex cs;
  • std::atomic tryLockChainTipScheduled{false};
  • std::atomic isEnabled{false};

And almost every other class in this PR.

Error message as reference:

error: use of deleted function ‘CBLSWorker::CBLSWorker(const CBLSWorker&)’
   92 |     const auto worker = blsWorker;
      |                         ^~~~~~~~~

./bls/bls_worker.h:20:7: note: ‘CBLSWorker::CBLSWorker(const CBLSWorker&)’ is implicitly deleted because the default definition would be ill-formed:
   20 | class CBLSWorker
      |       ^~~~~~~~~~
./bls/bls_worker.h: At global scope:
./bls/bls_worker.h:20:7: error: ‘ctpl::thread_pool::thread_pool(const ctpl::thread_pool&)’ is private within this context
In file included from ./bls/bls_worker.h:10:
./ctpl_stl.h:206:9: note: declared private here
  206 |         thread_pool(const thread_pool &);// = delete;
      |         ^~~~~~~~~~~
./bls/bls_worker.h:20:7: error: use of deleted function ‘std::mutex::mutex(const std::mutex&)’
   20 | class CBLSWorker
      |       ^~~~~~~~~~
In file included from /usr/include/c++/15/bits/atomic_wait.h:53,
                 from /usr/include/c++/15/bits/atomic_base.h:43,
                 from /usr/include/c++/15/atomic:52,
                 from ./serialize.h:12,
                 from ./script/script.h:12,
                 from ./primitives/transaction.h:11,
                 from ./llmq/commitment.h:8,
                 from ./llmq/dkgsession.h:8:
/usr/include/c++/15/bits/std_mutex.h:109:5: note: declared here
  109 |     mutex(const mutex&) = delete;
      |     ^~~~~

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware but I'd like this the non-copyable property to be inherent to the class definition and not reliant on the state of its members that are subject to change.

CBLSWorker& operator=(CBLSWorker const&) = delete;
CBLSWorker();
~CBLSWorker();

Expand Down
3 changes: 3 additions & 0 deletions src/chainlock/chainlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
std::atomic<std::chrono::seconds> lastCleanupTime{0s};

public:
CChainLocksHandler() = delete;
CChainLocksHandler(const CChainLocksHandler&) = delete;
CChainLocksHandler& operator=(const CChainLocksHandler&) = delete;
explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSporkManager& sporkman,
CTxMemPool& _mempool, const CMasternodeSync& mn_sync);
~CChainLocksHandler();
Expand Down
3 changes: 3 additions & 0 deletions src/chainlock/signing.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ class ChainLockSigner final : public llmq::CRecoveredSigsListener
uint256 lastSignedMsgHash GUARDED_BY(cs_signer);

public:
ChainLockSigner() = delete;
ChainLockSigner(const ChainLockSigner&) = delete;
ChainLockSigner& operator=(const ChainLockSigner&) = delete;
explicit ChainLockSigner(CChainState& chainstate, ChainLockSignerParent& clhandler, llmq::CSigningManager& sigman,
llmq::CSigSharesManager& shareman, CSporkManager& sporkman, const CMasternodeSync& mn_sync);
~ChainLockSigner();
Expand Down
62 changes: 55 additions & 7 deletions src/coinjoin/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ using wallet::CoinType;
using wallet::CWallet;
using wallet::ReserveDestination;

CCoinJoinClientQueueManager::CCoinJoinClientQueueManager(CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync) :
m_walletman{walletman},
m_dmnman{dmnman},
m_mn_metaman{mn_metaman},
m_mn_sync{mn_sync}
{
}

CCoinJoinClientQueueManager::~CCoinJoinClientQueueManager() = default;

MessageProcessingResult CCoinJoinClientQueueManager::ProcessMessage(NodeId from, CConnman& connman,
std::string_view msg_type, CDataStream& vRecv)
{
Expand Down Expand Up @@ -139,6 +150,28 @@ MessageProcessingResult CCoinJoinClientQueueManager::ProcessMessage(NodeId from,
return ret;
}

void CCoinJoinClientQueueManager::DoMaintenance()
{
if (!m_mn_sync.IsBlockchainSynced() || ShutdownRequested()) return;

CheckQueue();
}

CCoinJoinClientManager::CCoinJoinClientManager(const std::shared_ptr<wallet::CWallet>& wallet,
CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet{wallet},
m_dmnman{dmnman},
m_mn_metaman{mn_metaman},
m_mn_sync{mn_sync},
m_isman{isman},
m_queueman{queueman}
{
}

CCoinJoinClientManager::~CCoinJoinClientManager() = default;

void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv)
{
if (!CCoinJoinClientOptions::IsEnabled()) return;
Expand Down Expand Up @@ -1816,13 +1849,6 @@ void CCoinJoinClientManager::UpdatedBlockTip(const CBlockIndex* pindex)
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight);
}

void CCoinJoinClientQueueManager::DoMaintenance()
{
if (!m_mn_sync.IsBlockchainSynced() || ShutdownRequested()) return;

CheckQueue();
}

void CCoinJoinClientManager::DoMaintenance(ChainstateManager& chainman, CConnman& connman, const CTxMemPool& mempool)
{
if (!CCoinJoinClientOptions::IsEnabled()) return;
Expand Down Expand Up @@ -1874,6 +1900,28 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
obj.pushKV("sessions", arrSessions);
}

CoinJoinWalletManager::CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CTxMemPool& mempool,
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_chainman{chainman},
m_dmnman{dmnman},
m_mn_metaman{mn_metaman},
m_mempool{mempool},
m_mn_sync{mn_sync},
m_isman{isman},
m_queueman{queueman}
{
}

CoinJoinWalletManager::~CoinJoinWalletManager()
{
LOCK(cs_wallet_manager_map);
for (auto& [wallet_name, cj_man] : m_wallet_manager_map) {
cj_man.reset();
}
}

void CoinJoinWalletManager::Add(const std::shared_ptr<CWallet>& wallet)
{
LOCK(cs_wallet_manager_map);
Expand Down
54 changes: 17 additions & 37 deletions src/coinjoin/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,14 @@ class CoinJoinWalletManager {
using wallet_name_cjman_map = std::map<const std::string, std::unique_ptr<CCoinJoinClientManager>>;

public:
CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
const CTxMemPool& mempool, const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_chainman(chainman),
m_dmnman(dmnman),
m_mn_metaman(mn_metaman),
m_mempool(mempool),
m_mn_sync(mn_sync),
m_isman{isman},
m_queueman(queueman)
{}

~CoinJoinWalletManager() {
LOCK(cs_wallet_manager_map);
for (auto& [wallet_name, cj_man] : m_wallet_manager_map) {
cj_man.reset();
}
}
CoinJoinWalletManager() = delete;
CoinJoinWalletManager(const CoinJoinWalletManager&) = delete;
CoinJoinWalletManager& operator=(const CoinJoinWalletManager&) = delete;
explicit CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CTxMemPool& mempool,
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman);
~CoinJoinWalletManager();

void Add(const std::shared_ptr<wallet::CWallet>& wallet) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map);
void DoMaintenance(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_wallet_manager_map);
Expand Down Expand Up @@ -235,14 +225,12 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
mutable Mutex cs_ProcessDSQueue;

public:
CCoinJoinClientQueueManager() = delete;
CCoinJoinClientQueueManager(const CCoinJoinClientQueueManager&) = delete;
CCoinJoinClientQueueManager& operator=(const CCoinJoinClientQueueManager&) = delete;
explicit CCoinJoinClientQueueManager(CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync) :
m_walletman(walletman),
m_dmnman(dmnman),
m_mn_metaman(mn_metaman),
m_mn_sync(mn_sync)
{
}
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync);
~CCoinJoinClientQueueManager();

[[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, CConnman& connman, std::string_view msg_type,
CDataStream& vRecv)
Expand Down Expand Up @@ -285,21 +273,13 @@ class CCoinJoinClientManager
bool fCreateAutoBackups{true}; // builtin support for automatic backups

CCoinJoinClientManager() = delete;
CCoinJoinClientManager(CCoinJoinClientManager const&) = delete;
CCoinJoinClientManager& operator=(CCoinJoinClientManager const&) = delete;

CCoinJoinClientManager(const CCoinJoinClientManager&) = delete;
CCoinJoinClientManager& operator=(const CCoinJoinClientManager&) = delete;
explicit CCoinJoinClientManager(const std::shared_ptr<wallet::CWallet>& wallet, CDeterministicMNManager& dmnman,
CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
const llmq::CInstantSendManager& isman,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet(wallet),
m_dmnman(dmnman),
m_mn_metaman(mn_metaman),
m_mn_sync(mn_sync),
m_isman{isman},
m_queueman(queueman)
{
}
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman);
~CCoinJoinClientManager();

Comment on lines 275 to 283
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Make cached height fields atomic to avoid data races

These are updated/read from different threads without a shared lock. Make them std::atomic.

-    int nCachedLastSuccessBlock{0};
+    std::atomic<int> nCachedLastSuccessBlock{0};
@@
-    int nCachedBlockHeight{0};
+    std::atomic<int> nCachedBlockHeight{0};

Pair with .cpp changes to use load()/store().

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/coinjoin/client.h around lines 278 to 286, change the cached height
member fields that are accessed from multiple threads to std::atomic<int> (add
#include <atomic> if not present) so reads/writes are atomic; then update the
corresponding .cpp to replace direct reads/writes with .load() and .store()
calls (or appropriate memory order if needed) wherever those cached height
fields are accessed, ensuring they are properly initialized and used without
surrounding shared locks.

void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);

Expand Down
8 changes: 8 additions & 0 deletions src/coinjoin/coinjoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ void CCoinJoinBaseSession::SetNull()
nTimeLastSuccessfulStep = GetTime();
}

CCoinJoinBaseManager::CCoinJoinBaseManager() = default;

CCoinJoinBaseManager::~CCoinJoinBaseManager() = default;

void CCoinJoinBaseManager::SetNull()
{
LOCK(cs_vecqueue);
Expand Down Expand Up @@ -394,6 +398,10 @@ bilingual_str CoinJoin::GetMessageByID(PoolMessage nMessageID)
}
}

CDSTXManager::CDSTXManager() = default;

CDSTXManager::~CDSTXManager() = default;

void CDSTXManager::AddDSTX(const CCoinJoinBroadcastTx& dstx)
{
AssertLockNotHeld(cs_mapdstx);
Expand Down
9 changes: 7 additions & 2 deletions src/coinjoin/coinjoin.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ class CCoinJoinBaseManager
void CheckQueue() EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue);

public:
CCoinJoinBaseManager() = default;
CCoinJoinBaseManager();
virtual ~CCoinJoinBaseManager();

int GetQueueSize() const EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue) { LOCK(cs_vecqueue); return vecCoinJoinQueue.size(); }
bool GetQueueItemAndTry(CCoinJoinQueue& dsqRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_vecqueue);
Expand Down Expand Up @@ -371,7 +372,11 @@ class CDSTXManager
std::map<uint256, CCoinJoinBroadcastTx> mapDSTX GUARDED_BY(cs_mapdstx);

public:
CDSTXManager() = default;
CDSTXManager(const CDSTXManager&) = delete;
CDSTXManager& operator=(const CDSTXManager&) = delete;
CDSTXManager();
~CDSTXManager();

void AddDSTX(const CCoinJoinBroadcastTx& dstx) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
CCoinJoinBroadcastTx GetDSTX(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);

Expand Down
21 changes: 21 additions & 0 deletions src/coinjoin/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,27 @@

#include <univalue.h>

CCoinJoinServer::CCoinJoinServer(ChainstateManager& chainman, CConnman& _connman, CDeterministicMNManager& dmnman,
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman) :
m_chainman{chainman},
connman{_connman},
m_dmnman{dmnman},
m_dstxman{dstxman},
m_mn_metaman{mn_metaman},
mempool{mempool},
m_peerman{peerman},
m_mn_activeman{mn_activeman},
m_mn_sync{mn_sync},
m_isman{isman},
vecSessionCollaterals{},
fUnitTest{false}
{
}

CCoinJoinServer::~CCoinJoinServer() = default;

MessageProcessingResult CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv)
{
if (!m_mn_sync.IsBlockchainSynced()) return {};
Expand Down
19 changes: 5 additions & 14 deletions src/coinjoin/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,14 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);

public:
CCoinJoinServer() = delete;
CCoinJoinServer(const CCoinJoinServer&) = delete;
CCoinJoinServer& operator=(const CCoinJoinServer&) = delete;
explicit CCoinJoinServer(ChainstateManager& chainman, CConnman& _connman, CDeterministicMNManager& dmnman,
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman) :
m_chainman(chainman),
connman(_connman),
m_dmnman(dmnman),
m_dstxman(dstxman),
m_mn_metaman(mn_metaman),
mempool(mempool),
m_peerman(peerman),
m_mn_activeman(mn_activeman),
m_mn_sync(mn_sync),
m_isman{isman},
vecSessionCollaterals(),
fUnitTest(false)
{}
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman);
~CCoinJoinServer();

[[nodiscard]] MessageProcessingResult ProcessMessage(CNode& pfrom, std::string_view msg_type, CDataStream& vRecv);

Expand Down
15 changes: 15 additions & 0 deletions src/dbwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -709,4 +709,19 @@ class CDBTransaction {
}
};

namespace util {
struct DbWrapperParams
{
const fs::path path{""};
const bool memory{false};
const bool wipe{false};
const size_t cache_size{1 << 20};
};

static inline std::unique_ptr<CDBWrapper> MakeDbWrapper(const DbWrapperParams& params)
{
return std::make_unique<CDBWrapper>(params.path, params.cache_size, params.memory, params.wipe);
}
} // namespace util

#endif // BITCOIN_DBWRAPPER_H
16 changes: 9 additions & 7 deletions src/dsnotificationinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,18 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman,
const ChainstateManager& chainman,
const std::unique_ptr<CDeterministicMNManager>& dmnman,
const std::unique_ptr<LLMQContext>& llmq_ctx) :
m_connman(connman),
m_dstxman(dstxman),
m_mn_sync(mn_sync),
m_govman(govman),
m_chainman(chainman),
m_dmnman(dmnman),
m_llmq_ctx(llmq_ctx)
m_connman{connman},
m_dstxman{dstxman},
m_mn_sync{mn_sync},
m_govman{govman},
m_chainman{chainman},
m_dmnman{dmnman},
m_llmq_ctx{llmq_ctx}
{
}

CDSNotificationInterface::~CDSNotificationInterface() = default;

void CDSNotificationInterface::InitializeCurrentBlockTip()
{
SynchronousUpdatedBlockTip(m_chainman.ActiveChain().Tip(), nullptr, m_chainman.ActiveChainstate().IsInitialBlockDownload());
Expand Down
5 changes: 4 additions & 1 deletion src/dsnotificationinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ struct LLMQContext;
class CDSNotificationInterface : public CValidationInterface
{
public:
CDSNotificationInterface() = delete;
CDSNotificationInterface(const CDSNotificationInterface&) = delete;
CDSNotificationInterface& operator=(const CDSNotificationInterface&) = delete;
explicit CDSNotificationInterface(CConnman& connman,
CDSTXManager& dstxman,
CMasternodeSync& mn_sync,
CGovernanceManager& govman,
const ChainstateManager& chainman,
const std::unique_ptr<CDeterministicMNManager>& dmnman,
const std::unique_ptr<LLMQContext>& llmq_ctx);
virtual ~CDSNotificationInterface() = default;
virtual ~CDSNotificationInterface();

// a small helper to initialize current block height in sub-modules on startup
void InitializeCurrentBlockTip();
Expand Down
6 changes: 3 additions & 3 deletions src/evo/chainhelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class CChainstateHelper
const llmq::CChainLocksHandler& clhandler;

public:
CChainstateHelper() = delete;
CChainstateHelper(const CChainstateHelper&) = delete;
CChainstateHelper& operator=(const CChainstateHelper&) = delete;
explicit CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman, CMNHFManager& mnhfman,
CGovernanceManager& govman, llmq::CInstantSendManager& isman,
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman,
Expand All @@ -44,9 +47,6 @@ class CChainstateHelper
const llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman);
~CChainstateHelper();

CChainstateHelper() = delete;
CChainstateHelper(const CChainstateHelper&) = delete;

/** Passthrough functions to CChainLocksHandler */
bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const;
bool HasChainLock(int nHeight, const uint256& blockHash) const;
Expand Down
Loading
Loading