Skip to content

test: add CoinJoin functional mixing coverage #7407

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

Open
PastaPastaPasta wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

test: add CoinJoin functional mixing coverage #7407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b0da0a
test: add CoinJoin mixing functional coverage
PastaPastaPasta Jul 4, 2026
e34706b
test: cover automatic CoinJoin denomination creation
PastaPastaPasta Jul 4, 2026
6c05d1f
test: expand CoinJoin mixing edge coverage
PastaPastaPasta Jul 4, 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
12 changes: 9 additions & 3 deletions src/coinjoin/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,13 @@ static int WinnersToSkip()
? 1 : 8;
}

static bool IsDisconnectRequested(const CConnman& connman, const CService& addr)
{
return connman.ForNode(addr, CConnman::AllNodes, [](const CNode* pnode) {
return pnode->fDisconnect.load();
});
}
Comment on lines +961 to +966

@thepastaclaw thepastaclaw Jul 4, 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: New IsDisconnectRequested silently drops the m_masternode_connection guard on all networks

CConnman::IsMasternodeOrDisconnectRequested (src/net.cpp:4895-4900) returns pnode->m_masternode_connection || pnode->fDisconnect. This exact combined predicate is used consistently in net.cpp's own masternode-connection bookkeeping (src/net.cpp:3456 in getPendingQuorumNodes, and the inverse at src/net.cpp:3528 in getConnectToDmn) to avoid repurposing a connection already dedicated as a masternode/quorum/probe connection.

This PR replaces the call sites in JoinExistingQueue (src/coinjoin/client.cpp:1007) and StartNewQueue (src/coinjoin/client.cpp:1077) with a new local free function IsDisconnectRequested that only checks pnode->fDisconnect, dropping the m_masternode_connection half. That is a real behavior change on mainnet/testnet/devnet, not just regtest: JoinExistingQueue/StartNewQueue will now proceed to AddPendingMasternode() and set pendingDsaRequest against a masternode we already have an outbound m_masternode_connection to (e.g. an LLMQ or probe connection). getConnectToDmn() still refuses to open a second socket to an already-connected address, so DSA/DSI/DSS/DSTX traffic will now be multiplexed over an existing LLMQ/probe connection instead of a dedicated CoinJoin one.

This is also bundled under a test:-prefixed commit (4b0da0a) with no rationale in the message or PR description beyond enabling the small regtest masternode set to reuse peers. Please either (a) restore the m_masternode_connection short-circuit and address the regtest reuse constraint another way, or (b) split this hunk into its own coinjoin: commit with a body explaining why dropping the guard is safe for message ordering, disconnect-on-session-end semantics, and PoSe accounting, and confirm with the CoinJoin owners.

source: ['claude']


bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman)
{
if (!CCoinJoinClientOptions::IsEnabled()) return false;
Expand Down Expand Up @@ -997,7 +1004,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized,

m_clientman.AddUsedMasternode(dmn->proTxHash);

if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) {
if (IsDisconnectRequested(connman, dmn->pdmnState->netInfo->GetPrimary())) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 4, 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.

P2 Badge Avoid joining queues from already-used masternodes

When a wallet receives a queue from a masternode it has already mixed with and the masternode connection is still open, this new check no longer filters it out; JoinExistingQueue() also never checks IsUsedMasternode() before calling AddUsedMasternode(). In that scenario the client can mix another round through the same masternode, undermining the -coinjoinrounds expectation that rounds use separate masternodes. Add an explicit used-masternode guard for queue joins if connections are meant to be reusable.

Useful? React with 👍 / 👎.

WalletCJLogPrint(m_wallet, /* Continued */
"CCoinJoinClientSession::JoinExistingQueue -- skipping connection, masternode=%s\n", dmn->proTxHash.ToString());
continue;
Expand Down Expand Up @@ -1067,7 +1074,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon
continue;
}

if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) {
if (IsDisconnectRequested(connman, dmn->pdmnState->netInfo->GetPrimary())) {
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- skipping connection, masternode=%s\n",
dmn->proTxHash.ToString());
nTries++;
Expand Down Expand Up @@ -1782,4 +1789,3 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
}
obj.pushKV("sessions", arrSessions);
}

7 changes: 7 additions & 0 deletions src/coinjoin/options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ void CCoinJoinClientOptions::SetRounds(int nRounds)
options.nCoinJoinRounds = nRounds;
}

void CCoinJoinClientOptions::SetRandomRounds(int nRandomRounds)
{
CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
options.nCoinJoinRandomRounds = nRandomRounds;
}

void CCoinJoinClientOptions::SetAmount(CAmount amount)
{
CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
Expand All @@ -68,6 +74,7 @@ void CCoinJoinClientOptions::Init()
instance.fCoinJoinMultiSession = gArgs.GetBoolArg("-coinjoinmultisession", DEFAULT_COINJOIN_MULTISESSION);
instance.nCoinJoinSessions = std::min(std::max((int)gArgs.GetIntArg("-coinjoinsessions", DEFAULT_COINJOIN_SESSIONS), MIN_COINJOIN_SESSIONS), MAX_COINJOIN_SESSIONS);
instance.nCoinJoinRounds = std::min(std::max((int)gArgs.GetIntArg("-coinjoinrounds", DEFAULT_COINJOIN_ROUNDS), MIN_COINJOIN_ROUNDS), MAX_COINJOIN_ROUNDS);
instance.nCoinJoinRandomRounds = std::min(std::max((int)gArgs.GetIntArg("-coinjoinrandomrounds", COINJOIN_RANDOM_ROUNDS), MIN_COINJOIN_RANDOM_ROUNDS), MAX_COINJOIN_RANDOM_ROUNDS);
instance.nCoinJoinAmount = std::min(std::max((int)gArgs.GetIntArg("-coinjoinamount", DEFAULT_COINJOIN_AMOUNT), MIN_COINJOIN_AMOUNT), MAX_COINJOIN_AMOUNT);
instance.nCoinJoinDenomsGoal = std::min(std::max((int)gArgs.GetIntArg("-coinjoindenomsgoal", DEFAULT_COINJOIN_DENOMS_GOAL), MIN_COINJOIN_DENOMS_GOAL), MAX_COINJOIN_DENOMS_GOAL);
instance.nCoinJoinDenomsHardCap = std::min(std::max((int)gArgs.GetIntArg("-coinjoindenomshardcap", DEFAULT_COINJOIN_DENOMS_HARDCAP), MIN_COINJOIN_DENOMS_HARDCAP), MAX_COINJOIN_DENOMS_HARDCAP);
Expand Down
3 changes: 3 additions & 0 deletions src/coinjoin/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ static constexpr int MIN_COINJOIN_DENOMS_GOAL = 10;
static constexpr int MIN_COINJOIN_DENOMS_HARDCAP = 10;
static constexpr int MAX_COINJOIN_SESSIONS = 10;
static constexpr int MAX_COINJOIN_ROUNDS = 16;
static constexpr int MIN_COINJOIN_RANDOM_ROUNDS = 0;
static constexpr int MAX_COINJOIN_DENOMS_GOAL = 100000;
static constexpr int MAX_COINJOIN_DENOMS_HARDCAP = 100000;
static constexpr int MAX_COINJOIN_AMOUNT = MAX_MONEY / COIN;
Expand Down Expand Up @@ -49,6 +50,7 @@ static constexpr int COINJOIN_KEYS_THRESHOLD_WARNING = 100;
static constexpr int COINJOIN_KEYS_THRESHOLD_STOP = 50;
// Pseudorandomly mix up to this many times in addition to base round count
static constexpr int COINJOIN_RANDOM_ROUNDS = 3;
static constexpr int MAX_COINJOIN_RANDOM_ROUNDS = COINJOIN_RANDOM_ROUNDS;
Comment on lines 52 to +53

@thepastaclaw thepastaclaw Jul 4, 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.

💬 Nitpick: -coinjoinrandomrounds can only decrease the default; help text does not convey the cap is tied to the default

MAX_COINJOIN_RANDOM_ROUNDS = COINJOIN_RANDOM_ROUNDS = 3 (src/coinjoin/options.h:52-53) plus the clamp in src/coinjoin/options.cpp mean -coinjoinrandomrounds can only be used to reduce randomness below the current default; users can never raise it. That is presumably intentional (the new functional test relies on -coinjoinrandomrounds=0), but the wallet-facing arg description in src/wallet/init.cpp (Mix up to N additional pseudorandom rounds beyond -coinjoinrounds (0-3, default: 3)) does not convey that the cap is a hard constant tied to the default. If this is really a test-only knob, consider marking it as debug/hidden rather than exposing it under OptionsCategory::WALLET_COINJOIN.

source: ['claude']


/* Application wide mixing options */
class CCoinJoinClientOptions
Expand All @@ -65,6 +67,7 @@ class CCoinJoinClientOptions
static void SetMultiSessionEnabled(bool fEnabled);
static void SetSessions(int sessions);
static void SetRounds(int nRounds);
static void SetRandomRounds(int nRandomRounds);
static void SetAmount(CAmount amount);
static void SetDenomsGoal(int denoms_goal);
static void SetDenomsHardCap(int denoms_hardcap);
Expand Down
1 change: 1 addition & 0 deletions src/wallet/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
argsman.AddArg("-coinjoindenomsgoal=<n>", strprintf("Try to create at least N inputs of each denominated amount (%u-%u, default: %u)", MIN_COINJOIN_DENOMS_GOAL, MAX_COINJOIN_DENOMS_GOAL, DEFAULT_COINJOIN_DENOMS_GOAL), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
argsman.AddArg("-coinjoindenomshardcap=<n>", strprintf("Create up to N inputs of each denominated amount (%u-%u, default: %u)", MIN_COINJOIN_DENOMS_HARDCAP, MAX_COINJOIN_DENOMS_HARDCAP, DEFAULT_COINJOIN_DENOMS_HARDCAP), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
argsman.AddArg("-coinjoinmultisession", strprintf("Enable multiple CoinJoin mixing sessions per block, experimental (0-1, default: %u)", DEFAULT_COINJOIN_MULTISESSION), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
argsman.AddArg("-coinjoinrandomrounds=<n>", strprintf("Mix up to N additional pseudorandom rounds beyond -coinjoinrounds (%u-%u, default: %u)", MIN_COINJOIN_RANDOM_ROUNDS, MAX_COINJOIN_RANDOM_ROUNDS, COINJOIN_RANDOM_ROUNDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
argsman.AddArg("-coinjoinrounds=<n>", strprintf("Use N separate masternodes for each denominated input to mix funds (%u-%u, default: %u)", MIN_COINJOIN_ROUNDS, MAX_COINJOIN_ROUNDS, DEFAULT_COINJOIN_ROUNDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
argsman.AddArg("-coinjoinsessions=<n>", strprintf("Use N separate masternodes in parallel to mix funds (%u-%u, default: %u)", MIN_COINJOIN_SESSIONS, MAX_COINJOIN_SESSIONS, DEFAULT_COINJOIN_SESSIONS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);

Expand Down
4 changes: 4 additions & 0 deletions src/wallet/test/coinjoin_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ BOOST_AUTO_TEST_CASE(coinjoin_options_tests)

CCoinJoinClientOptions::SetRounds(DEFAULT_COINJOIN_ROUNDS + 10);
BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetRounds(), DEFAULT_COINJOIN_ROUNDS + 10);
CCoinJoinClientOptions::SetRandomRounds(MIN_COINJOIN_RANDOM_ROUNDS);
BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetRandomRounds(), MIN_COINJOIN_RANDOM_ROUNDS);
CCoinJoinClientOptions::SetRandomRounds(COINJOIN_RANDOM_ROUNDS);
BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetRandomRounds(), COINJOIN_RANDOM_ROUNDS);
CCoinJoinClientOptions::SetAmount(DEFAULT_COINJOIN_AMOUNT + 50);
BOOST_CHECK_EQUAL(CCoinJoinClientOptions::GetAmount(), DEFAULT_COINJOIN_AMOUNT + 50);
}
Expand Down
Loading
Loading