Skip to content

feat: share vecMasternodesUsed across all wallets, improve its handling #6875

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 4 commits into from
Oct 28, 2025
Merged

feat: share vecMasternodesUsed across all wallets, improve its handling #6875

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c82532
feat: share `vecMasternodesUsed` across all wallets, improve its hand…
UdjinM6 Oct 4, 2025
13a28fa
perf: optimize CoinJoin masternode tracking with hybrid data structure
UdjinM6 Oct 6, 2025
f50d09a
fix: update `MasternodeMetaStore::ToString()`
UdjinM6 Oct 10, 2025
eb3be09
refactor: code style/log text adjustments
UdjinM6 Oct 26, 2025
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
32 changes: 17 additions & 15 deletions src/coinjoin/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ void CCoinJoinClientSession::ResetPool()
void CCoinJoinClientManager::ResetPool()
{
nCachedLastSuccessBlock = 0;
vecMasternodesUsed.clear();
AssertLockNotHeld(cs_deqsessions);
LOCK(cs_deqsessions);
for (auto& session : deqSessions) {
Expand Down Expand Up @@ -967,11 +966,15 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
// If we've used 90% of the Masternode list then drop the oldest first ~30%
int nThreshold_high = nMnCountEnabled * 0.9;
int nThreshold_low = nThreshold_high * 0.7;
WalletCJLogPrint(m_wallet, "Checking vecMasternodesUsed: size: %d, threshold: %d\n", (int)vecMasternodesUsed.size(), nThreshold_high);
size_t nUsedMasternodes{m_mn_metaman.GetUsedMasternodesCount()};

if ((int)vecMasternodesUsed.size() > nThreshold_high) {
vecMasternodesUsed.erase(vecMasternodesUsed.begin(), vecMasternodesUsed.begin() + vecMasternodesUsed.size() - nThreshold_low);
WalletCJLogPrint(m_wallet, " vecMasternodesUsed: new size: %d, threshold: %d\n", (int)vecMasternodesUsed.size(), nThreshold_high);
WalletCJLogPrint(m_wallet, "Checking nUsedMasternodes: %d, threshold: %d\n", (int)nUsedMasternodes, nThreshold_high);
Copy link
Copy Markdown
Collaborator

@knst knst Oct 26, 2025

Choose a reason for hiding this comment

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

nit(code-style):

nUsedMasternodes is not matched with our code style.

prefix n should not be used.
case of variable should be snake-case, such as used_masternodes or used_masternodes_count or something similar.

See doc/developer-notes.md:

  • Variable (including function arguments) and namespace names are all lowercase and may use _ to
    separate words (snake_case).

I'd suggest to change log record to be a bit more human rather than use variable name in logs (even if it's debug):

Suggested change
WalletCJLogPrint(m_wallet, "Checking nUsedMasternodes: %d, threshold: %d\n", (int)nUsedMasternodes, nThreshold_high);
WalletCJLogPrint(m_wallet, "Checking threshold of used masternodes for coinjoin: amount=%d, threshold=%d\n", (int)nUsedMasternodes, nThreshold_high);


if ((int)nUsedMasternodes > nThreshold_high) {
size_t nToRemove{nUsedMasternodes - nThreshold_low};
m_mn_metaman.RemoveUsedMasternodes(nToRemove);
WalletCJLogPrint(m_wallet, " new nUsedMasternodes: %d, threshold: %d\n",
(int)m_mn_metaman.GetUsedMasternodesCount(), nThreshold_high);
Comment thread
UdjinM6 marked this conversation as resolved.
Outdated
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Comment thread
UdjinM6 marked this conversation as resolved.
bool fResult = true;
Expand All @@ -995,17 +998,17 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
return fResult;
}

void CCoinJoinClientManager::AddUsedMasternode(const COutPoint& outpointMn)
void CCoinJoinClientManager::AddUsedMasternode(const uint256& proTxHash)
{
vecMasternodesUsed.push_back(outpointMn);
m_mn_metaman.AddUsedMasternode(proTxHash);
}

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

size_t nCountEnabled = mnList.GetValidMNsCount();
size_t nCountNotExcluded = nCountEnabled - vecMasternodesUsed.size();
size_t nCountNotExcluded{nCountEnabled - m_mn_metaman.GetUsedMasternodesCount()};

Comment on lines +1010 to 1011
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Oct 26, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix potential size_t underflow and printf specifiers.

If used_count > enabled, subtraction underflows; also %d mismatches size_t. Clamp and cast.

-    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);
+    const size_t used_count = m_mn_metaman.GetUsedMasternodesCount();
+    const size_t nCountNotExcluded = nCountEnabled > used_count ? (nCountEnabled - used_count) : 0;
+
+    WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %d enabled masternodes, %d masternodes to choose from\n",
+                     __func__, static_cast<int>(nCountEnabled), static_cast<int>(nCountNotExcluded));

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

🤖 Prompt for AI Agents 
In src/coinjoin/client.cpp around lines 1010-1011, the subtraction size_t
nCountNotExcluded{nCountEnabled - m_mn_metaman.GetUsedMasternodesCount()}; can
underflow if GetUsedMasternodesCount() > nCountEnabled and the printf formatting
uses wrong specifier; change to compute a non-negative value by clamping to zero
(e.g. compute used = m_mn_metaman.GetUsedMasternodesCount(); nCountNotExcluded =
(used >= nCountEnabled) ? 0 : (nCountEnabled - used);), and update any
printf/format calls to use the correct size_t specifier (%zu) or cast to
unsigned/uint64_t and use the matching specifier to avoid mismatches.

WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %d enabled masternodes, %d masternodes to choose from\n", __func__, nCountEnabled, nCountNotExcluded);
if (nCountNotExcluded < 1) {
Expand All @@ -1022,15 +1025,14 @@ CDeterministicMNCPtr CCoinJoinClientManager::GetRandomNotUsedMasternode()
// shuffle pointers
Shuffle(vpMasternodesShuffled.begin(), vpMasternodesShuffled.end(), FastRandomContext());

std::set<COutPoint> excludeSet(vecMasternodesUsed.begin(), vecMasternodesUsed.end());

// loop through
// loop through - using direct O(1) lookup instead of creating a set copy
for (const auto& dmn : vpMasternodesShuffled) {
if (excludeSet.count(dmn->collateralOutpoint)) {
if (m_mn_metaman.IsUsedMasternode(dmn->proTxHash)) {
continue;
}

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

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

m_clientman.AddUsedMasternode(dsq.masternodeOutpoint);
m_clientman.AddUsedMasternode(dmn->proTxHash);

if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) {
WalletCJLogPrint(m_wallet, /* Continued */
Expand Down Expand Up @@ -1137,7 +1139,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon
return false;
}

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

// skip next mn payments winners
if (dmn->pdmnState->nLastPaidHeight + nWeightedMnCount < mnList.GetHeight() + WinnersToSkip()) {
Expand Down
5 changes: 1 addition & 4 deletions src/coinjoin/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,6 @@ class CCoinJoinClientManager
const llmq::CInstantSendManager& m_isman;
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> ??
std::deque<CCoinJoinClientSession> deqSessions GUARDED_BY(cs_deqsessions);
Expand Down Expand Up @@ -327,7 +324,7 @@ class CCoinJoinClientManager

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

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

void UpdatedSuccessBlock();
Expand Down
38 changes: 36 additions & 2 deletions src/masternode/meta.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <univalue.h>
#include <util/time.h>

const std::string MasternodeMetaStore::SERIALIZATION_VERSION_STRING = "CMasternodeMetaMan-Version-4";
const std::string MasternodeMetaStore::SERIALIZATION_VERSION_STRING = "CMasternodeMetaMan-Version-5";

CMasternodeMetaMan::CMasternodeMetaMan() :
m_db{std::make_unique<db_type>("mncache.dat", "magicMasternodeCache")}
Expand Down Expand Up @@ -153,10 +153,44 @@ void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBa
m_seen_platform_bans.insert(inv_hash, std::move(msg));
}

void CMasternodeMetaMan::AddUsedMasternode(const uint256& proTxHash)
{
LOCK(cs);
// Only add if not already present (prevents duplicates)
if (m_used_masternodes_set.insert(proTxHash).second) {
m_used_masternodes.push_back(proTxHash);
}
}

void CMasternodeMetaMan::RemoveUsedMasternodes(size_t nCount)
Copy link
Copy Markdown
Collaborator

@knst knst Oct 26, 2025

Choose a reason for hiding this comment

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

nit: nCount -> count

Suggested change
void CMasternodeMetaMan::RemoveUsedMasternodes(size_t nCount)
void CMasternodeMetaMan::RemoveUsedMasternodes(size_t count)

{
LOCK(cs);
size_t removed = 0;
while (removed < nCount && !m_used_masternodes.empty()) {
Copy link
Copy Markdown
Collaborator

@knst knst Oct 26, 2025

Choose a reason for hiding this comment

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

Suggested change
while (removed < nCount && !m_used_masternodes.empty()) {
while (removed < count && !m_used_masternodes.empty()) {

// Remove from both the set and the deque
m_used_masternodes_set.erase(m_used_masternodes.front());
m_used_masternodes.pop_front();
++removed;
}
}

size_t CMasternodeMetaMan::GetUsedMasternodesCount() const
{
LOCK(cs);
return m_used_masternodes.size();
}

bool CMasternodeMetaMan::IsUsedMasternode(const uint256& proTxHash) const
{
LOCK(cs);
return m_used_masternodes_set.find(proTxHash) != m_used_masternodes_set.end();
}

std::string MasternodeMetaStore::ToString() const
{
LOCK(cs);
return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d", metaInfos.size(), nDsqCount);
return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d, used masternodes count: %d",
metaInfos.size(), nDsqCount, m_used_masternodes.size());
}

uint256 PlatformBanMessage::GetHash() const { return ::SerializeHash(*this); }
26 changes: 24 additions & 2 deletions src/masternode/meta.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <unordered_lru_cache.h>

#include <atomic>
#include <deque>
#include <map>
#include <memory>
#include <optional>
Expand Down Expand Up @@ -139,6 +140,10 @@ class MasternodeMetaStore
std::map<uint256, CMasternodeMetaInfoPtr> metaInfos GUARDED_BY(cs);
// keep track of dsq count to prevent masternodes from gaming coinjoin queue
std::atomic<int64_t> nDsqCount{0};
// keep track of the used Masternodes for CoinJoin across all wallets
// Using deque for efficient FIFO removal and unordered_set for O(1) lookups
std::deque<uint256> m_used_masternodes GUARDED_BY(cs);
Uint256HashSet m_used_masternodes_set GUARDED_BY(cs);

public:
template<typename Stream>
Expand All @@ -149,7 +154,9 @@ class MasternodeMetaStore
for (const auto& p : metaInfos) {
tmpMetaInfo.emplace_back(*p.second);
}
s << SERIALIZATION_VERSION_STRING << tmpMetaInfo << nDsqCount;
// Convert deque to vector for serialization - unordered_set will be rebuilt on deserialization
std::vector<uint256> tmpUsedMasternodes(m_used_masternodes.begin(), m_used_masternodes.end());
s << SERIALIZATION_VERSION_STRING << tmpMetaInfo << nDsqCount << tmpUsedMasternodes;
}

template<typename Stream>
Expand All @@ -164,18 +171,27 @@ class MasternodeMetaStore
return;
}
std::vector<CMasternodeMetaInfo> tmpMetaInfo;
s >> tmpMetaInfo >> nDsqCount;
std::vector<uint256> tmpUsedMasternodes;
s >> tmpMetaInfo >> nDsqCount >> tmpUsedMasternodes;

metaInfos.clear();
for (auto& mm : tmpMetaInfo) {
metaInfos.emplace(mm.GetProTxHash(), std::make_shared<CMasternodeMetaInfo>(std::move(mm)));
}

// Convert vector to deque and build unordered_set for O(1) lookups
m_used_masternodes.assign(tmpUsedMasternodes.begin(), tmpUsedMasternodes.end());
m_used_masternodes_set.clear();
m_used_masternodes_set.insert(tmpUsedMasternodes.begin(), tmpUsedMasternodes.end());
}

void Clear() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);

metaInfos.clear();
m_used_masternodes.clear();
m_used_masternodes_set.clear();
}

std::string ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
Expand Down Expand Up @@ -257,6 +273,12 @@ class CMasternodeMetaMan : public MasternodeMetaStore
bool AlreadyHavePlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
std::optional<PlatformBanMessage> GetPlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) EXCLUSIVE_LOCKS_REQUIRED(!cs);

// CoinJoin masternode tracking
void AddUsedMasternode(const uint256& proTxHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
void RemoveUsedMasternodes(size_t nCount) EXCLUSIVE_LOCKS_REQUIRED(!cs);
size_t GetUsedMasternodesCount() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool IsUsedMasternode(const uint256& proTxHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
};

#endif // BITCOIN_MASTERNODE_META_H
Loading