test: cover use_cj success path in rpc_coinjoin.py - #7637
Conversation
The use_cj tests added in dash#7261 only exercise failure paths: the test wallet never holds a fully mixed coin, so nothing verifies that a successful use_cj spend actually selects mixed inputs, suppresses change, or records the DS="1" CoinJoin marker that FinishTransaction now attaches via CommitTransaction. A regression in any of those would pass the suite unnoticed. Build two fully mixed coins by simulating mixing rounds with same-denomination fee-free self-spends (mined directly, since zero-fee transactions are not relayed) up to COINJOIN_ROUNDS_MIN + COINJOIN_RANDOM_ROUNDS rounds, which makes IsFullyMixed() deterministic regardless of the wallet salt. Then assert listunspent rounds and the anonymized balance, a send with a fully mixed preset input (single input, no change, DS="1") and a sendall sweep of the remaining mixed coin (DS="1").
|
🕓 Ready for review — next in queue (commit 2821f84) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe functional test adds precise amount handling and a three-round CoinJoin constant. It simulates mined self-spends for equal-denomination outputs. The test verifies mixed balances and checks that Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds coverage for successful CoinJoin spending behavior without changing production code; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 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 |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The exact-head diff adds focused success-path coverage for fully mixed CoinJoin inputs, change suppression, automatic selection, anonymized balance accounting, and the DS marker. Source inspection found no correctness or reliability defects, and rpc_coinjoin.py passed with both legacy and descriptor wallets; Python compilation and diff whitespace checks also passed.
Source: Codex reviewer backend model (exact model ID not supplied in the evidence); Claude verifier backend model (exact model ID not exposed in the runtime context). openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and is not reviewer evidence.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
| inputs = [{'txid': funding_txid, 'vout': out['n']} for out in funding_tx['vout'] if out['value'] == denom] | ||
| assert_equal(len(inputs), 2) | ||
|
|
||
| # Simulate mixing: a same-denomination, fee-free self-spend advances each |
There was a problem hiding this comment.
it's a good way to simulate mixing, I think. Let's make it a helper to use it in this functional test for other scenarios.
There was a problem hiding this comment.
Done in 2821f84. I extracted the round-advancing self-spend into a reusable simulate_mixing(...) helper and kept the scenario-specific funding and assertions in test_use_cj_success.
🤖 Posted autonomously by Codex on behalf of pasta.
Issue being fixed or feature implemented
Follow-up to #7261. The
use_cjtests added there only exercise failure paths: the test wallet never holds a fully mixed coin, so nothing verifies that a successfuluse_cjspend actually selects mixed inputs, suppresses change, or records theDS="1"CoinJoin marker thatFinishTransaction()now attaches viaCommitTransaction(). A regression in any of those would pass the suite unnoticed.What was done?
Added a success-path subtest to
test/functional/rpc_coinjoin.py. It builds two fully mixed coins by simulating mixing: same-denomination, fee-free self-spends advance a denominated output by one round each, and they are mined directly viagenerateblocksince zero-fee transactions are not relayed. ChainingCOINJOIN_ROUNDS_MIN + COINJOIN_RANDOM_ROUNDSrounds makesIsFullyMixed()deterministic regardless of the wallet's salt (the salt-based coin flip only applies strictly below that threshold).It then asserts:
coinjoin_roundsinlistunspentand count towards thegetbalancesanonymized balance;sendaccepts a fully mixed preset input (the positive counterpart of the rejection tests from feat: add use_cj feature to RPCs send, sendall and fundrawtransaction #7261), spends exactly that input, creates no change output (the remainder is paid as fee, as fully mixed coins are spent in whole denominations) and recordsDS="1";sendallwith automatic selection sweeps exactly the remaining fully mixed coin and recordsDS="1".setcoinjoinroundsis a node-global setting; the subtest pins it toCOINJOIN_ROUNDS_MINand restores the default at the end so later subtests are unaffected.How Has This Been Tested?
test/functional/test_runner.py rpc_coinjoin.pypasses locally (macOS arm64,--enable-debugbuild, both descriptor and legacy wallet paths of the runner's default).test/lint/lint-python.pyclean.Breaking Changes
None, test-only change.
Checklist: