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/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ BITCOIN_CORE_H = \
clientversion.h \
coinjoin/coinjoin.h \
coinjoin/client.h \
coinjoin/context.h \
coinjoin/options.h \
coinjoin/server.h \
coinjoin/util.h \
Expand Down Expand Up @@ -391,6 +392,7 @@ libbitcoin_server_a_SOURCES = \
blockfilter.cpp \
chain.cpp \
coinjoin/coinjoin.cpp \
coinjoin/context.cpp \
coinjoin/options.cpp \
coinjoin/server.cpp \
consensus/tx_verify.cpp \
Expand Down
343 changes: 171 additions & 172 deletions src/coinjoin/client.cpp

Large diffs are not rendered by default.

93 changes: 66 additions & 27 deletions src/coinjoin/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,31 @@

#include <coinjoin/util.h>
#include <coinjoin/coinjoin.h>
#include <util/check.h>
#include <util/translation.h>

#include <atomic>
#include <deque>
#include <memory>
#include <utility>

class CDeterministicMN;
using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;

class CBlockPolicyEstimator;
class CCoinJoinClientManager;
class CCoinJoinClientQueueManager;

class CBlockPolicyEstimator;
class CConnman;
class CDeterministicMN;
class CJClientManager;
class CNode;
class CMasternodeSync;
class CTxMemPool;
class PeerManager;

class UniValue;
class CMasternodeSync;

using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;

// The main object for accessing mixing
extern std::map<const std::string, std::shared_ptr<CCoinJoinClientManager>> coinJoinClientManagers;

// The object to track mixing queues
extern std::unique_ptr<CCoinJoinClientQueueManager> coinJoinClientQueueManager;
extern std::unique_ptr<CJClientManager> coinJoinClientManagers;

class CPendingDsaRequest
{
Expand Down Expand Up @@ -72,10 +70,53 @@ class CPendingDsaRequest
}
};

class CJClientManager {
public:
using wallet_name_cjman_map = std::map<const std::string, std::unique_ptr<CCoinJoinClientManager>>;

public:
CJClientManager(CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman)
: m_connman(connman), m_mempool(mempool), m_mn_sync(mn_sync), m_queueman(queueman) {}
~CJClientManager() {
for (auto& [wallet_name, cj_man] : m_wallet_manager_map) {
cj_man.reset();
}
}

void Add(CWallet& wallet);
void DoMaintenance(CBlockPolicyEstimator& fee_estimator);

void Remove(const std::string& name) {
m_wallet_manager_map.erase(name);
}

CCoinJoinClientManager* Get(const CWallet& wallet) const {
auto it = m_wallet_manager_map.find(wallet.GetName());
return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr;
}

const wallet_name_cjman_map& raw() const { return m_wallet_manager_map; }

private:
CConnman& m_connman;
Comment thread
knst marked this conversation as resolved.
Outdated
CTxMemPool& m_mempool;

const CMasternodeSync& m_mn_sync;
const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;

wallet_name_cjman_map m_wallet_manager_map;
};

class CCoinJoinClientSession : public CCoinJoinBaseSession
{
private:
CWallet& m_wallet;
CJClientManager& m_clientman;
CCoinJoinClientManager& m_manager;

const CMasternodeSync& m_mn_sync;
const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;

std::vector<COutPoint> vecOutPointLocked;

Expand All @@ -88,8 +129,6 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession

CKeyHolderStorage keyHolderStorage; // storage for keys used in PrepareDenominate

CWallet& mixingWallet;

/// Create denominations
bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate);
bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals);
Expand Down Expand Up @@ -125,10 +164,9 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
void SetNull() EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);

public:
explicit CCoinJoinClientSession(CWallet& pwallet, const CMasternodeSync& mn_sync) :
m_mn_sync(mn_sync), mixingWallet(pwallet)
{
}
explicit CCoinJoinClientSession(CWallet& pwallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet(pwallet), m_clientman(clientman), m_manager(*Assert(clientman.Get(pwallet))), m_mn_sync(mn_sync), m_queueman(queueman) {}

void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);

Expand Down Expand Up @@ -159,12 +197,13 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
{
private:
CConnman& connman;
CJClientManager& m_clientman;
const CMasternodeSync& m_mn_sync;
mutable Mutex cs_ProcessDSQueue;

public:
explicit CCoinJoinClientQueueManager(CConnman& _connman, const CMasternodeSync& mn_sync) :
connman(_connman), m_mn_sync(mn_sync) {};
explicit CCoinJoinClientQueueManager(CConnman& _connman, CJClientManager& clientman, const CMasternodeSync& mn_sync) :
connman(_connman), m_clientman(clientman), m_mn_sync(mn_sync) {};

void ProcessMessage(const CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_vecqueue);
void ProcessDSQueue(const CNode& peer, PeerManager& peerman, CDataStream& vRecv);
Expand All @@ -176,10 +215,14 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
class CCoinJoinClientManager
{
private:
// Keep track of the used Masternodes
std::vector<COutPoint> vecMasternodesUsed;
CWallet& m_wallet;
CJClientManager& m_clientman;

const CMasternodeSync& m_mn_sync;
const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;

// Keep track of the used Masternodes
std::vector<COutPoint> vecMasternodesUsed;

mutable Mutex cs_deqsessions;
// TODO: or map<denom, CCoinJoinClientSession> ??
Expand All @@ -191,8 +234,6 @@ class CCoinJoinClientManager
int nMinBlocksToWait{1}; // how many blocks to wait for after one successful mixing tx in non-multisession mode
bilingual_str strAutoDenomResult;

CWallet& mixingWallet;

// Keep track of current block height
int nCachedBlockHeight{0};

Expand All @@ -209,8 +250,9 @@ class CCoinJoinClientManager
CCoinJoinClientManager(CCoinJoinClientManager const&) = delete;
CCoinJoinClientManager& operator=(CCoinJoinClientManager const&) = delete;

explicit CCoinJoinClientManager(CWallet& wallet, const CMasternodeSync& mn_sync) :
m_mn_sync(mn_sync), mixingWallet(wallet) {}
explicit CCoinJoinClientManager(CWallet& wallet, CJClientManager& clientman, const CMasternodeSync& mn_sync,
const std::unique_ptr<CCoinJoinClientQueueManager>& queueman) :
m_wallet(wallet), m_clientman(clientman), m_mn_sync(mn_sync), m_queueman(queueman) {}

void ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) LOCKS_EXCLUDED(cs_deqsessions);

Expand Down Expand Up @@ -246,7 +288,4 @@ class CCoinJoinClientManager
void GetJsonInfo(UniValue& obj) const LOCKS_EXCLUDED(cs_deqsessions);
};


void DoCoinJoinMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool);

#endif // BITCOIN_COINJOIN_CLIENT_H
34 changes: 34 additions & 0 deletions src/coinjoin/context.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) 2023 The Dash Core developers
// Distributed under the MIT/X11 software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <coinjoin/context.h>

#include <net.h>
#include <policy/fees.h>
#include <txmempool.h>

#ifdef ENABLE_WALLET
#include <coinjoin/client.h>
#endif // ENABLE_WALLET
#include <coinjoin/server.h>

CJContext::CJContext(CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, bool relay_txes) :
#ifdef ENABLE_WALLET
clientman {
[&]() -> CJClientManager* const {
assert(::coinJoinClientManagers == nullptr);
::coinJoinClientManagers = std::make_unique<CJClientManager>(connman, mempool, mn_sync, queueman);
return ::coinJoinClientManagers.get();
}()
},
queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *clientman, mn_sync) : nullptr},
#endif // ENABLE_WALLET
server{std::make_unique<CCoinJoinServer>(chainstate, connman, mempool, mn_sync)}
{}

CJContext::~CJContext() {
#ifdef ENABLE_WALLET
::coinJoinClientManagers.reset();
#endif // ENABLE_WALLET
}
39 changes: 39 additions & 0 deletions src/coinjoin/context.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) 2023 The Dash Core developers
// Distributed under the MIT/X11 software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_COINJOIN_CONTEXT_H
#define BITCOIN_COINJOIN_CONTEXT_H

#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif

#include <memory>

class CBlockPolicyEstimator;
class CChainState;
class CCoinJoinServer;
class CConnman;
class CMasternodeSync;
class CTxMemPool;

#ifdef ENABLE_WALLET
class CCoinJoinClientQueueManager;
class CJClientManager;
#endif // ENABLE_WALLET

struct CJContext {
CJContext() = delete;
Comment thread
knst marked this conversation as resolved.
Outdated
CJContext(const CJContext&) = delete;
CJContext(CChainState& chainstate, CConnman& connman, CTxMemPool& mempool, const CMasternodeSync& mn_sync, bool relay_txes);
~CJContext();

#ifdef ENABLE_WALLET
CJClientManager* const clientman;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could just make this a shared_pointer and make the global a shared pointer (they point to the same object) to ensure that we don't get dangling pointers?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is continuation of precedent set with LLMQContext's quorum_block_processor, qman, clhandler and isman being bare pointers (source), which in turn is a continuation of precedent set by NodeContext (source).

NodeContext uses bare pointers to indicate they're not responsible for management of the pointers, LLMQContext and others, which do use the struct for initialization, use it to indicate the lack of exclusive control.

But yes, the concern of dangling pointers is present, one could reset the global pointer container, with CJContext having the faintest idea but switching positions would mean that CJContext (or anything that has access to its members) could perform a reset and code that relies on the global alias wouldn't have a clue about it.

Both cases assume heavily erroneous code and neither case prevents the other. The switch can be done but I don't see the benefit.

const std::unique_ptr<CCoinJoinClientQueueManager> queueman;
#endif // ENABLE_WALLET
const std::unique_ptr<CCoinJoinServer> server;
};

#endif // BITCOIN_COINJOIN_CONTEXT_H
11 changes: 4 additions & 7 deletions src/coinjoin/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

#include <univalue.h>

std::unique_ptr<CCoinJoinServer> coinJoinServer;
constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10};

void CCoinJoinServer::ProcessMessage(CNode& peer, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv)
Expand Down Expand Up @@ -886,17 +885,15 @@ void CCoinJoinServer::SetState(PoolState nStateNew)
nState = nStateNew;
}

void CCoinJoinServer::DoMaintenance() const
void CCoinJoinServer::DoMaintenance()
{
if (!fMasternodeMode) return; // only run on masternodes
if (!m_mn_sync.IsBlockchainSynced()) return;
if (ShutdownRequested()) return;

if (!coinJoinServer) return;

coinJoinServer->CheckForCompleteQueue();
coinJoinServer->CheckPool();
coinJoinServer->CheckTimeout();
CheckForCompleteQueue();
CheckPool();
CheckTimeout();
}

void CCoinJoinServer::GetJsonInfo(UniValue& obj) const
Expand Down
5 changes: 1 addition & 4 deletions src/coinjoin/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ class PeerManager;

class UniValue;

// The main object for accessing mixing
extern std::unique_ptr<CCoinJoinServer> coinJoinServer;

/** Used to keep track of current status of mixing pool
*/
class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
Expand Down Expand Up @@ -96,7 +93,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
void CheckTimeout();
void CheckForCompleteQueue();

void DoMaintenance() const;
void DoMaintenance();

void GetJsonInfo(UniValue& obj) const;
};
Expand Down
8 changes: 5 additions & 3 deletions src/dsnotificationinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#ifdef ENABLE_WALLET
#include <coinjoin/client.h>
#endif // ENABLE_WALLET
#include <coinjoin/context.h>
#include <dsnotificationinterface.h>
#include <governance/governance.h>
#include <masternode/sync.h>
Expand All @@ -23,8 +24,9 @@

CDSNotificationInterface::CDSNotificationInterface(CConnman& _connman,
CMasternodeSync& _mn_sync, const std::unique_ptr<CDeterministicMNManager>& _dmnman,
CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx
) : connman(_connman), m_mn_sync(_mn_sync), dmnman(_dmnman), govman(_govman), llmq_ctx(_llmq_ctx) {}
CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx,
const std::unique_ptr<CJContext>& _cj_ctx
) : connman(_connman), m_mn_sync(_mn_sync), dmnman(_dmnman), govman(_govman), llmq_ctx(_llmq_ctx), cj_ctx(_cj_ctx) {}

void CDSNotificationInterface::InitializeCurrentBlockTip()
{
Expand Down Expand Up @@ -66,7 +68,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con

CCoinJoin::UpdatedBlockTip(pindexNew, *llmq_ctx->clhandler, m_mn_sync);
#ifdef ENABLE_WALLET
for (auto& pair : coinJoinClientManagers) {
for (auto& pair : cj_ctx->clientman->raw()) {
pair.second->UpdatedBlockTip(pindexNew);
}
#endif // ENABLE_WALLET
Expand Down
5 changes: 4 additions & 1 deletion src/dsnotificationinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ class CConnman;
class CDeterministicMNManager;
class CGovernanceManager;
class CMasternodeSync;
struct CJContext;
struct LLMQContext;

class CDSNotificationInterface : public CValidationInterface
{
public:
explicit CDSNotificationInterface(CConnman& _connman,
CMasternodeSync& _mn_sync, const std::unique_ptr<CDeterministicMNManager>& _dmnman,
CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx);
CGovernanceManager& _govman, const std::unique_ptr<LLMQContext>& _llmq_ctx,
const std::unique_ptr<CJContext>& _cj_ctx);
virtual ~CDSNotificationInterface() = default;

// a small helper to initialize current block height in sub-modules on startup
Expand All @@ -45,6 +47,7 @@ class CDSNotificationInterface : public CValidationInterface
CGovernanceManager& govman;

const std::unique_ptr<LLMQContext>& llmq_ctx;
const std::unique_ptr<CJContext>& cj_ctx;
};

#endif // BITCOIN_DSNOTIFICATIONINTERFACE_H
4 changes: 2 additions & 2 deletions src/dummywallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class DummyWalletInit : public WalletInitInterface {

// Dash Specific WalletInitInterface InitCoinJoinSettings
void AutoLockMasternodeCollaterals() const override {}
void InitCoinJoinSettings() const override {}
void InitCoinJoinSettings(const CJClientManager& clientman) const override {}
bool InitAutoBackup() const override {return true;}
};

Expand Down Expand Up @@ -74,7 +74,7 @@ const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();

namespace interfaces {

std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet, const CJClientManager& clientman)
{
throw std::logic_error("Wallet function called in non-wallet build.");
}
Expand Down
Loading