Skip to content

refactor: couple simplifications for coinjoin's client implementation #7470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 5 commits into from
Jul 22, 2026
Merged

refactor: couple simplifications for coinjoin's client implementation #7470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
844519e
refactor: inline AddUsedMasternode to CCoinJoinClientSession
knst Jul 10, 2026
ea70b34
refactor: move GetRandomNotUsedMasternode from CCoinJoinClientManager…
knst Jul 10, 2026
b968667
refactor: move nCachedNumBlocks which is used by qt OverviewScreen to…
knst Jul 10, 2026
d68d8c4
refactor: simplify logging for cj client and removes unused strAutoDe…
knst Jul 13, 2026
a19dee0
refactor: make nMinBlocksToWait as constexpr static instead member va…
knst Jul 13, 2026
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
58 changes: 20 additions & 38 deletions src/coinjoin/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,7 @@ void CCoinJoinClientManager::CheckTimeout()

LOCK(cs_deqsessions);
for (auto& session : deqSessions) {
if (session.CheckTimeout()) {
strAutoDenomResult = _("Session timed out.");
}
session.CheckTimeout();
}
}

Expand Down Expand Up @@ -611,40 +609,33 @@ bool CCoinJoinClientManager::CheckAutomaticBackup()

switch (nWalletBackups) {
case 0:
strAutoDenomResult = _("Automatic backups disabled") + Untranslated(", ") + _("no mixing available.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- Automatic backups disabled, no mixing available.\n");
stopMixing();
m_wallet->nKeysLeftSinceAutoBackup = 0; // no backup, no "keys since last backup"
return false;
case -1:
// Automatic backup failed, nothing else we can do until user fixes the issue manually.
// There is no way to bring user attention in daemon mode, so we just update status and
// There is no way to bring user attention in daemon mode, so we just
// keep spamming if debug is on.
strAutoDenomResult = _("ERROR! Failed to create automatic backup") + Untranslated(", ") + _("see debug.log for details.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- ERROR! Failed to create automatic backup, see debug.log for details.\n");
return false;
case -2:
// We were able to create automatic backup but keypool was not replenished because wallet is locked.
// There is no way to bring user attention in daemon mode, so we just update status and
// There is no way to bring user attention in daemon mode, so we just
// keep spamming if debug is on.
strAutoDenomResult = _("WARNING! Failed to replenish keypool, please unlock your wallet to do so.") + Untranslated(", ") + _("see debug.log for details.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- WARNING! Failed to replenish keypool, please unlock your wallet to do so, see debug.log for details.\n");
return false;
}

if (m_wallet->nKeysLeftSinceAutoBackup < COINJOIN_KEYS_THRESHOLD_STOP) {
// We should never get here via mixing itself but probably something else is still actively using keypool
strAutoDenomResult = strprintf(_("Very low number of keys left: %d") + Untranslated(", ") +
_("no mixing available."),
m_wallet->nKeysLeftSinceAutoBackup);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- Very low number of keys left: %d, no mixing available.\n", m_wallet->nKeysLeftSinceAutoBackup);
// It's getting really dangerous, stop mixing
stopMixing();
return false;
} else if (m_wallet->nKeysLeftSinceAutoBackup < COINJOIN_KEYS_THRESHOLD_WARNING) {
// Low number of keys left, but it's still more or less safe to continue
strAutoDenomResult = strprintf(_("Very low number of keys left: %d"), m_wallet->nKeysLeftSinceAutoBackup);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- Very low number of keys left: %d\n", m_wallet->nKeysLeftSinceAutoBackup);

if (fCreateAutoBackups) {
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- Trying to create new backup.\n");
Expand All @@ -658,8 +649,7 @@ bool CCoinJoinClientManager::CheckAutomaticBackup()
}
if (!errorString.original.empty()) {
// Things are really broken
strAutoDenomResult = _("ERROR! Failed to create automatic backup") + Untranslated(": ") + errorString;
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- ERROR! Failed to create automatic backup: %s\n", errorString.original);
return false;
}
}
Expand Down Expand Up @@ -864,12 +854,12 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
if (!CCoinJoinClientOptions::IsEnabled() || !isMixing()) return false;

if (!m_mn_sync.IsBlockchainSynced()) {
strAutoDenomResult = _("Can't mix while sync in progress.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::DoAutomaticDenominating -- Can't mix while sync in progress.\n");
return false;
}

if (!fDryRun && m_wallet->IsLocked(true)) {
strAutoDenomResult = _("Wallet is locked.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::DoAutomaticDenominating -- Wallet is locked.\n");
return false;
}

Expand Down Expand Up @@ -898,8 +888,7 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
if (!CheckAutomaticBackup()) return false;

if (WaitForAnotherBlock()) {
strAutoDenomResult = _("Last successful action was too recent.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::DoAutomaticDenominating -- %s\n", strAutoDenomResult.original);
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::DoAutomaticDenominating -- Last successful action was too recent.\n");
return false;
}

Expand All @@ -909,19 +898,14 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
return fResult;
}

void CCoinJoinClientManager::AddUsedMasternode(const uint256& proTxHash)
{
m_mn_metaman.AddUsedMasternode(proTxHash);
}

CDeterministicMNCPtr CCoinJoinClientManager::GetRandomNotUsedMasternode()
CDeterministicMNCPtr CCoinJoinClientSession::GetRandomNotUsedMasternode()
{
auto mnList = m_dmnman.GetListAtChainTip();

size_t nCountEnabled = mnList.GetCounts().enabled();
size_t nCountNotExcluded{nCountEnabled - m_mn_metaman.GetUsedMasternodesCount()};

WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %d enabled masternodes, %d masternodes to choose from\n", __func__, nCountEnabled, nCountNotExcluded);
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- %d enabled masternodes, %d masternodes to choose from\n", __func__, nCountEnabled, nCountNotExcluded);
if (nCountNotExcluded < 1) {
return nullptr;
}
Expand All @@ -941,12 +925,12 @@ CDeterministicMNCPtr CCoinJoinClientManager::GetRandomNotUsedMasternode()
continue;
}

WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- found, masternode=%s\n", __func__,
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- found, masternode=%s\n", __func__,
dmn->proTxHash.ToString());
return dmn;
}

WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- failed\n", __func__);
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- failed\n", __func__);
return nullptr;
}

Expand Down Expand Up @@ -994,7 +978,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized,
continue;
}

m_clientman.AddUsedMasternode(dmn->proTxHash);
m_mn_metaman.AddUsedMasternode(dmn->proTxHash);

if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) {
WalletCJLogPrint(m_wallet, /* Continued */
Expand Down Expand Up @@ -1042,14 +1026,14 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon

// otherwise, try one randomly
while (nTries < 10) {
auto dmn = m_clientman.GetRandomNotUsedMasternode();
auto dmn = GetRandomNotUsedMasternode();
if (!dmn) {
strAutoDenomResult = _("Can't find random Masternode.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- %s\n", strAutoDenomResult.original);
return false;
}

m_clientman.AddUsedMasternode(dmn->proTxHash);
m_mn_metaman.AddUsedMasternode(dmn->proTxHash);

// skip next mn payments winners
if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) {
Expand Down Expand Up @@ -1138,9 +1122,7 @@ void CCoinJoinClientManager::ProcessPendingDsaRequest(CConnman& connman)
AssertLockNotHeld(cs_deqsessions);
LOCK(cs_deqsessions);
for (auto& session : deqSessions) {
if (session.ProcessPendingDsaRequest(connman)) {
strAutoDenomResult = _("Mixing in progress…");
}
session.ProcessPendingDsaRequest(connman);
}
}

Expand Down
12 changes: 3 additions & 9 deletions src/coinjoin/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession

bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);
bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);
CDeterministicMNCPtr GetRandomNotUsedMasternode();

/// step 0: select denominated inputs and txouts
bool SelectDenominate(std::string& strErrorRet, std::vector<CTxDSIn>& vecTxDSInRet);
Expand Down Expand Up @@ -170,13 +171,12 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client
std::deque<CCoinJoinClientSession> deqSessions GUARDED_BY(cs_deqsessions);

int nCachedLastSuccessBlock{0};
int nMinBlocksToWait{1}; // how many blocks to wait for after one successful mixing tx in non-multisession mode
bilingual_str strAutoDenomResult;
// how many blocks to wait for after one successful mixing tx in non-multisession mode
static constexpr int nMinBlocksToWait{1};

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

int nCachedNumBlocks{std::numeric_limits<int>::max()}; // used for the overview screen
bool fCreateAutoBackups{true}; // builtin support for automatic backups

bool WaitForAnotherBlock() const;
Expand Down Expand Up @@ -209,9 +209,6 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client

void ProcessPendingDsaRequest(CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);

void AddUsedMasternode(const uint256& proTxHash);
CDeterministicMNCPtr GetRandomNotUsedMasternode();

void UpdatedSuccessBlock();

void UpdatedBlockTip(const CBlockIndex* pindex);
Expand All @@ -220,9 +217,6 @@ class CCoinJoinClientManager : public interfaces::CoinJoin::Client
EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);

// interfaces::CoinJoin::Client overrides
void resetCachedBlocks() override { nCachedNumBlocks = std::numeric_limits<int>::max(); }
int getCachedBlocks() const override { return nCachedNumBlocks; }
void setCachedBlocks(int nCachedBlocks) override { nCachedNumBlocks = nCachedBlocks; }
void disableAutobackups() override { fCreateAutoBackups = false; }
void resetPool() override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
UniValue getJsonInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
Expand Down
3 changes: 0 additions & 3 deletions src/interfaces/coinjoin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@ class Client
{
public:
virtual ~Client() {}
virtual void resetCachedBlocks() = 0;
virtual void resetPool() = 0;
virtual int getCachedBlocks() const = 0;
virtual UniValue getJsonInfo() const = 0;
virtual std::vector<std::string> getSessionStatuses() const = 0;
virtual std::string getSessionDenoms() const = 0;
virtual void setCachedBlocks(int nCachedBlocks) = 0;
virtual void disableAutobackups() = 0;
virtual bool isMixing() const = 0;
virtual bool startMixing() = 0;
Expand Down
1 change: 0 additions & 1 deletion src/qt/optionsdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,6 @@ void OptionsDialog::on_okButton_clicked()
#ifdef ENABLE_WALLET
if (m_enable_wallet) {
for (auto& wallet : model->node().walletLoader().getWallets()) {
model->node().coinJoinLoader()->WithClient(wallet->getWalletName(), [](auto& client) { client.resetCachedBlocks(); });
wallet->markDirty();
}
}
Expand Down
41 changes: 18 additions & 23 deletions src/qt/overviewpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <interfaces/coinjoin.h>

#include <algorithm>
#include <limits>
#include <map>
#include <cmath>

Expand Down Expand Up @@ -518,6 +519,10 @@ void OverviewPage::coinJoinStatus(bool fForce)

if (!fForce && (clientModel->node().shutdownRequested() || !clientModel->masternodeSync().isBlockchainSynced())) return;

// Forced updates are triggered by mode changes (advanced UI, CoinJoin or privacy toggled),
// the status has to be re-rendered even if the block height didn't change
if (fForce) nCachedNumBlocks = std::numeric_limits<int>::max();
Comment on lines +522 to +524

@thepastaclaw thepastaclaw Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Suggestion: Commit b968667 mixes a labeled refactor with an undisclosed behavior fix

Commit b968667 is titled as moving nCachedNumBlocks into OverviewPage, but its diff also adds the new if (fForce) nCachedNumBlocks = std::numeric_limits<int>::max(); behavior that fixes stale progress after an Advanced CoinJoin UI toggle. Because Dash merges the commit stack without squashing, please amend the commit message to disclose the forced-invalidation fix so future blame and bisect users do not encounter a behavior change hidden behind a pure-refactor subject.

source: ['claude']


// Disable any PS UI for masternode or when autobackup is disabled or failed for whatever reason
if (clientModel->node().isMasternode() || nWalletBackups <= 0) {
DisableCoinJoinCompletely();
Expand Down Expand Up @@ -577,11 +582,7 @@ void OverviewPage::coinJoinStatus(bool fForce)
int nBestHeight = clientModel->node().getNumBlocks();

// We are processing more than 1 block per second, we'll just leave
bool tooFast{false};
walletModel->withCoinJoin([&](auto& client) {
tooFast = nBestHeight > client.getCachedBlocks() && GetTime() - nLastDSProgressBlockTime <= 1;
});
if (tooFast) return;
if (nBestHeight > nCachedNumBlocks && GetTime() - nLastDSProgressBlockTime <= 1) return;
nLastDSProgressBlockTime = GetTime();

QString strKeysLeftText(tr("keys left: %1").arg(walletModel->getKeysLeftSinceAutoBackup()));
Expand All @@ -596,15 +597,11 @@ void OverviewPage::coinJoinStatus(bool fForce)

QString strCoinJoinName = QString::fromStdString(gCoinJoinName);
bool notMixing{false};
bool refreshProgress{false};
walletModel->withCoinJoin([&](auto& client) {
notMixing = !client.isMixing();
if (notMixing && nBestHeight != client.getCachedBlocks()) {
client.setCachedBlocks(nBestHeight);
refreshProgress = true;
}
});
if (refreshProgress) updateCoinJoinProgress();
walletModel->withCoinJoin([&](auto& client) { notMixing = !client.isMixing(); });
if (notMixing && nBestHeight != nCachedNumBlocks) {
nCachedNumBlocks = nBestHeight;
updateCoinJoinProgress();
}
if (notMixing) {
setWidgetsVisible(false);
ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName));
Expand Down Expand Up @@ -688,16 +685,14 @@ void OverviewPage::coinJoinStatus(bool fForce)
}

// check coinjoin status and unlock if needed
bool refreshProgressTail{false};
if (nBestHeight != nCachedNumBlocks) {
// Balance and number of transactions might have changed
nCachedNumBlocks = nBestHeight;
updateCoinJoinProgress();
}
walletModel->withCoinJoin([&](auto& client) {
if (nBestHeight != client.getCachedBlocks()) {
// Balance and number of transactions might have changed
client.setCachedBlocks(nBestHeight);
refreshProgressTail = true;
}
ui->labelSubmittedDenom->setText(m_privacy ? "####" : QString(client.getSessionDenoms().c_str()));
});
if (refreshProgressTail) updateCoinJoinProgress();

setWidgetsVisible(true);
}
Expand Down Expand Up @@ -734,7 +729,7 @@ void OverviewPage::toggleCoinJoin(){
if(!ctx.isValid())
{
//unlock was cancelled
walletModel->withCoinJoin([](auto& client) { client.resetCachedBlocks(); });
nCachedNumBlocks = std::numeric_limits<int>::max();
QMessageBox::warning(this, strCoinJoinName,
tr("Wallet is locked and user declined to unlock. Disabling %1.").arg(strCoinJoinName),
QMessageBox::Ok, QMessageBox::Ok);
Expand All @@ -745,8 +740,8 @@ void OverviewPage::toggleCoinJoin(){

}

nCachedNumBlocks = std::numeric_limits<int>::max();
walletModel->withCoinJoin([&](auto& client) {
client.resetCachedBlocks();
if (client.isMixing()) {
ui->toggleCoinJoin->setText(tr("Start %1").arg(strCoinJoinName));
client.resetPool();
Expand Down
3 changes: 3 additions & 0 deletions src/qt/overviewpage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <QFont>
#include <QWidget>

#include <limits>
#include <memory>

class ClientModel;
Expand Down Expand Up @@ -58,6 +59,8 @@ public Q_SLOTS:
BitcoinUnit m_display_bitcoin_unit;
bool fShowAdvancedCJUI;
int cachedNumISLocks{-1};
//! Block height at which the CoinJoin status was last rendered
int nCachedNumBlocks{std::numeric_limits<int>::max()};

TxViewDelegate *txdelegate;
std::unique_ptr<TransactionFilterProxy> filter;
Expand Down
Loading