test: add CoinJoin functional mixing coverage#7407
Conversation
|
|
✅ Review complete (commit 6c05d1f) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c05d1f1a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| m_clientman.AddUsedMasternode(dmn->proTxHash); | ||
|
|
||
| if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) { | ||
| if (IsDisconnectRequested(connman, dmn->pdmnState->netInfo->GetPrimary())) { |
There was a problem hiding this comment.
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 👍 / 👎.
WalkthroughThis PR modifies CoinJoin client disconnect-checking logic to use a new Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Wallet1
participant Wallet2
participant Masternode
participant Mempool
Wallet1->>Masternode: start mixing, join/start queue
Wallet2->>Masternode: start mixing, join queue
Masternode-->>Wallet1: session/queue established
Masternode-->>Wallet2: session/queue established
Wallet1->>Mempool: submit CoinJoin transaction
Wallet2->>Mempool: submit CoinJoin transaction
Mempool-->>Wallet1: joint mixing tx observed
Mempool-->>Wallet2: joint mixing tx observed
Wallet1->>Wallet1: confirm mixed outputs, verify anonymized balance
Wallet2->>Wallet2: confirm mixed outputs, verify anonymized balance
Compact metadata:
Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/coinjoin/options.cpp (1)
84-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
GetJsonInfodoesn't expose the newrandom_roundssetting.
GetJsonInfosurfacesmax_sessions,max_rounds,max_amount,denoms_goal, anddenoms_hardcap, but the newly addednCoinJoinRandomRoundsoption is not included. This leaves the new CLI knob invisible to any RPC consumer inspecting CoinJoin config state.♻️ Proposed fix
obj.pushKV("max_rounds", options.nCoinJoinRounds.load()); + obj.pushKV("max_random_rounds", options.nCoinJoinRandomRounds.load()); obj.pushKV("max_amount", options.nCoinJoinAmount.load());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/coinjoin/options.cpp` around lines 84 - 95, CCoinJoinClientOptions::GetJsonInfo is missing the new random_rounds field, so RPC consumers cannot see that setting. Update the JSON export in GetJsonInfo to include the nCoinJoinRandomRounds value alongside the existing max_sessions, max_rounds, max_amount, denoms_goal, and denoms_hardcap entries, using a clear key name that matches the CLI option.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/functional/feature_coinjoin_mixing.py`:
- Around line 548-596: The fee check in spend_mixed_funds is using the same
destination address for both the partial spend and the final spend, so
getreceivedbyaddress(address) includes earlier receipts and corrupts the fee
calculation. Update spend_mixed_funds to use a fresh address for the final
send-to-address call, or otherwise isolate the final transaction’s receive total
before computing fee. Keep the existing assertions around getreceivedbyaddress
and fee, but make them refer only to the final transaction’s destination so the
fee sanity check is meaningful.
---
Nitpick comments:
In `@src/coinjoin/options.cpp`:
- Around line 84-95: CCoinJoinClientOptions::GetJsonInfo is missing the new
random_rounds field, so RPC consumers cannot see that setting. Update the JSON
export in GetJsonInfo to include the nCoinJoinRandomRounds value alongside the
existing max_sessions, max_rounds, max_amount, denoms_goal, and denoms_hardcap
entries, using a clear key name that matches the CLI option.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 33bb6931-e0c4-4bd5-b87f-aa1269301dd4
📒 Files selected for processing (7)
src/coinjoin/client.cppsrc/coinjoin/options.cppsrc/coinjoin/options.hsrc/wallet/init.cppsrc/wallet/test/coinjoin_tests.cpptest/functional/feature_coinjoin_mixing.pytest/functional/test_runner.py
| def spend_mixed_funds(self): | ||
| self.log.info("Spend mixed funds with use_cj") | ||
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | ||
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | ||
| assert_greater_than(len(mixed_utxos), 1) | ||
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | ||
| address = self.nodes[0].getnewaddress() | ||
| assert_raises_rpc_error( | ||
| -6, | ||
| "Unable to locate enough mixed funds for this transaction.", | ||
| self.w1.sendtoaddress, | ||
| address=address, | ||
| amount=anonymized + min(mixed_utxos.values()), | ||
| subtractfeefromamount=True, | ||
| use_cj=True, | ||
| ) | ||
|
|
||
| partial_amount = min(mixed_utxos.values()) | ||
| partial_txid = self.w1.sendtoaddress(address=address, amount=partial_amount, subtractfeefromamount=True, use_cj=True) | ||
| partial_tx = self.w1.getrawtransaction(partial_txid, True) | ||
| partial_spent = Decimal("0") | ||
| for txin in partial_tx['vin']: | ||
| outpoint = (txin['txid'], txin['vout']) | ||
| assert outpoint in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | ||
| partial_spent += mixed_utxos[outpoint] | ||
|
|
||
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | ||
| self.bump_mocktime(1) | ||
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | ||
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], anonymized - partial_spent) | ||
|
|
||
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | ||
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | ||
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | ||
| txid = self.w1.sendtoaddress(address=address, amount=anonymized, subtractfeefromamount=True, use_cj=True) | ||
|
|
||
| # Only fully mixed inputs may fund this transaction | ||
| tx = self.w1.getrawtransaction(txid, True) | ||
| for txin in tx['vin']: | ||
| assert (txin['txid'], txin['vout']) in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | ||
|
|
||
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | ||
| self.bump_mocktime(1) | ||
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | ||
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], 0) | ||
| received = self.nodes[0].getreceivedbyaddress(address) | ||
| fee = anonymized - received | ||
| assert_greater_than(received, 0) | ||
| assert_greater_than(Decimal("0.001"), fee) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Address reuse corrupts the fee sanity check.
address (line 554) is reused for both the partial spend (line 566) and the final full spend (line 582). getreceivedbyaddress(address) at line 593 therefore sums receipts from both transactions, but fee = anonymized - received (line 594) only accounts for the second transaction's target amount. This makes received inflated by partial_spent, driving fee strongly negative, so assert_greater_than(Decimal("0.001"), fee) (line 596) passes trivially regardless of the real transaction fee — the intended fee-size assertion is not actually being validated.
🐛 Proposed fix: use a fresh address for the final spend
mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount']
for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET}
anonymized = self.w1.getbalances()['mine']['coinjoin']
- txid = self.w1.sendtoaddress(address=address, amount=anonymized, subtractfeefromamount=True, use_cj=True)
+ final_address = self.nodes[0].getnewaddress()
+ txid = self.w1.sendtoaddress(address=final_address, amount=anonymized, subtractfeefromamount=True, use_cj=True)
# Only fully mixed inputs may fund this transaction
tx = self.w1.getrawtransaction(txid, True)
for txin in tx['vin']:
assert (txin['txid'], txin['vout']) in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}"
self.sync_mempools([self.nodes[0], self.w1, self.w2])
self.bump_mocktime(1)
self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks())
assert_equal(self.w1.getbalances()['mine']['coinjoin'], 0)
- received = self.nodes[0].getreceivedbyaddress(address)
+ received = self.nodes[0].getreceivedbyaddress(final_address)
fee = anonymized - received
assert_greater_than(received, 0)
assert_greater_than(Decimal("0.001"), fee)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def spend_mixed_funds(self): | |
| self.log.info("Spend mixed funds with use_cj") | |
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | |
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | |
| assert_greater_than(len(mixed_utxos), 1) | |
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | |
| address = self.nodes[0].getnewaddress() | |
| assert_raises_rpc_error( | |
| -6, | |
| "Unable to locate enough mixed funds for this transaction.", | |
| self.w1.sendtoaddress, | |
| address=address, | |
| amount=anonymized + min(mixed_utxos.values()), | |
| subtractfeefromamount=True, | |
| use_cj=True, | |
| ) | |
| partial_amount = min(mixed_utxos.values()) | |
| partial_txid = self.w1.sendtoaddress(address=address, amount=partial_amount, subtractfeefromamount=True, use_cj=True) | |
| partial_tx = self.w1.getrawtransaction(partial_txid, True) | |
| partial_spent = Decimal("0") | |
| for txin in partial_tx['vin']: | |
| outpoint = (txin['txid'], txin['vout']) | |
| assert outpoint in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | |
| partial_spent += mixed_utxos[outpoint] | |
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | |
| self.bump_mocktime(1) | |
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | |
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], anonymized - partial_spent) | |
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | |
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | |
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | |
| txid = self.w1.sendtoaddress(address=address, amount=anonymized, subtractfeefromamount=True, use_cj=True) | |
| # Only fully mixed inputs may fund this transaction | |
| tx = self.w1.getrawtransaction(txid, True) | |
| for txin in tx['vin']: | |
| assert (txin['txid'], txin['vout']) in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | |
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | |
| self.bump_mocktime(1) | |
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | |
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], 0) | |
| received = self.nodes[0].getreceivedbyaddress(address) | |
| fee = anonymized - received | |
| assert_greater_than(received, 0) | |
| assert_greater_than(Decimal("0.001"), fee) | |
| def spend_mixed_funds(self): | |
| self.log.info("Spend mixed funds with use_cj") | |
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | |
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | |
| assert_greater_than(len(mixed_utxos), 1) | |
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | |
| address = self.nodes[0].getnewaddress() | |
| assert_raises_rpc_error( | |
| -6, | |
| "Unable to locate enough mixed funds for this transaction.", | |
| self.w1.sendtoaddress, | |
| address=address, | |
| amount=anonymized + min(mixed_utxos.values()), | |
| subtractfeefromamount=True, | |
| use_cj=True, | |
| ) | |
| partial_amount = min(mixed_utxos.values()) | |
| partial_txid = self.w1.sendtoaddress(address=address, amount=partial_amount, subtractfeefromamount=True, use_cj=True) | |
| partial_tx = self.w1.getrawtransaction(partial_txid, True) | |
| partial_spent = Decimal("0") | |
| for txin in partial_tx['vin']: | |
| outpoint = (txin['txid'], txin['vout']) | |
| assert outpoint in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | |
| partial_spent += mixed_utxos[outpoint] | |
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | |
| self.bump_mocktime(1) | |
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | |
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], anonymized - partial_spent) | |
| mixed_utxos = {(utxo['txid'], utxo['vout']): utxo['amount'] | |
| for utxo in self.w1.listunspent() if utxo['coinjoin_rounds'] >= MIXING_ROUNDS_TARGET} | |
| anonymized = self.w1.getbalances()['mine']['coinjoin'] | |
| final_address = self.nodes[0].getnewaddress() | |
| txid = self.w1.sendtoaddress(address=final_address, amount=anonymized, subtractfeefromamount=True, use_cj=True) | |
| # Only fully mixed inputs may fund this transaction | |
| tx = self.w1.getrawtransaction(txid, True) | |
| for txin in tx['vin']: | |
| assert (txin['txid'], txin['vout']) in mixed_utxos, f"spent a non-mixed input: {txin['txid']}:{txin['vout']}" | |
| self.sync_mempools([self.nodes[0], self.w1, self.w2]) | |
| self.bump_mocktime(1) | |
| self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks()) | |
| assert_equal(self.w1.getbalances()['mine']['coinjoin'], 0) | |
| received = self.nodes[0].getreceivedbyaddress(final_address) | |
| fee = anonymized - received | |
| assert_greater_than(received, 0) | |
| assert_greater_than(Decimal("0.001"), fee) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/functional/feature_coinjoin_mixing.py` around lines 548 - 596, The fee
check in spend_mixed_funds is using the same destination address for both the
partial spend and the final spend, so getreceivedbyaddress(address) includes
earlier receipts and corrupts the fee calculation. Update spend_mixed_funds to
use a fresh address for the final send-to-address call, or otherwise isolate the
final transaction’s receive total before computing fee. Keep the existing
assertions around getreceivedbyaddress and fee, but make them refer only to the
final transaction’s destination so the fee sanity check is meaningful.
There was a problem hiding this comment.
Code Review
Solid new functional CoinJoin mixing test, plus small additive test hooks (SetRandomRounds/-coinjoinrandomrounds). The one non-test change worth calling out: src/coinjoin/client.cpp replaces CConnman::IsMasternodeOrDisconnectRequested (m_masternode_connection || fDisconnect) with a local helper that only checks fDisconnect. Both agents converged on this being a silent production semantic change bundled under a test-only commit message; it also leaves the original CConnman method dead. No blocking correctness issue proven, but the change deserves explicit justification or scoping.
Source: reviewers opus for general, claude-sonnet-5 for general, gpt-5.5[high] for general (failed), opus for dash-core-commit-history, claude-sonnet-5 for dash-core-commit-history, gpt-5.5[high] for dash-core-commit-history (failed); verifier opus.
🟡 1 suggestion(s) | 💬 2 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/coinjoin/client.cpp`:
- [SUGGESTION] src/coinjoin/client.cpp:961-966: 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 (4b0da0a2b4) 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.
In `src/net.h`:
- [NITPICK] src/net.h:1356: CConnman::IsMasternodeOrDisconnectRequested is now dead code
After the switch to the local `IsDisconnectRequested` helper in src/coinjoin/client.cpp, `CConnman::IsMasternodeOrDisconnectRequested` has zero callers (only the declaration at src/net.h:1356 and definition at src/net.cpp:4895). Either remove it as part of this change or, if the intent is to keep it as available API, add a comment explaining why — otherwise future callers will reintroduce the pre-PR behavior inconsistently.
In `src/coinjoin/options.h`:
- [NITPICK] src/coinjoin/options.h:52-53: -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`.
| static bool IsDisconnectRequested(const CConnman& connman, const CService& addr) | ||
| { | ||
| return connman.ForNode(addr, CConnman::AllNodes, [](const CNode* pnode) { | ||
| return pnode->fDisconnect.load(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 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']
| static constexpr int COINJOIN_RANDOM_ROUNDS = 3; | ||
| static constexpr int MAX_COINJOIN_RANDOM_ROUNDS = COINJOIN_RANDOM_ROUNDS; |
There was a problem hiding this comment.
💬 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']
2 participants
Issue being fixed or feature implemented
Adds functional CoinJoin coverage for real end-to-end mixing on regtest. This covers the wallet/client/server behavior that is difficult to prove with unit tests alone, including queue/session flow through masternodes, automatic denomination setup, restart recovery, reorg bookkeeping, and spending mixed funds.
What was done?
feature_coinjoin_mixing.pyto exercise two wallet nodes mixing through a regtest masternode set.use_cj=truespend coverage, including overspend rejection, partial mixed spend accounting, and final fully mixed spend selection.This pull request was created by Codex.
How Has This Been Tested?
Ran:
make -j8 src/dashd src/dash-cli src/test/test_dashtest/functional/feature_coinjoin_mixing.py --portseed=8888env TEST_RUNNER_PORT_MIN=32000 test/functional/feature_coinjoin_mixing.py --portseed=1test/functional/rpc_coinjoin.py --portseed=9001test/functional/p2p_dstx.py --portseed=9002src/test/test_dash --run_test=coinjoin_tests/coinjoin_options_testspython3 -m py_compile test/functional/feature_coinjoin_mixing.pygit diff --checktest/lint/lint-python.pywas attempted but skipped becauseflake8is not installed in this environment.Breaking Changes
None.
Checklist: