fix: preserve wallet permissions when changing passphrases

[PastaPastaPasta]

Issue being fixed or feature implemented

A successful walletpassphrasechange during a CoinJoin mixing-only unlock silently left the wallet fully unlocked. An already authenticated caller with signing/spending/export permission could use keys beyond the owner's prior mixing restriction. This does not bypass RPC authentication, and the existing unlock timer still bounds the interval.

What was done?

Capture the original locked/mixing mode under the wallet's existing locks. Unlock a mixing-only wallet as mixing-only so it stays that way, and relock wallets that started locked on every exit path via a cleanup handler. Successful maintenance preserves full unlock where it was already allowed; incorrect-old-passphrase behavior, encryption format and unlock timers remain unchanged.

How Has This Been Tested?

Built on macOS arm64 with prebuilt depends. The added regression fails against baseline in both wallet backends because signing succeeds after changing a mixing-only wallet's passphrase. wallet_encryption.py --legacy-wallet and --descriptors pass with the fix.

The regression runs a locked/mixing/full input table: after walletpassphrasechange, it checks signmessage (with signature verification) and coinjoin start/stop authorization for each mode. Existing encryption, passphrase-change, null-byte and timeout-bound coverage in run_test is unchanged. Ran test_dash --run_test=bip39_tests,wallet_crypto_tests, Python lint, whitespace lint and git diff --check.

Breaking Changes

None. Successful passphrase changes retain the existing wallet permissions.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: aa1ed758-1f2d-47a4-a9ff-b0873863b25a

📥 Commits

Reviewing files that changed from the base of the PR and between 41310f4 and c4305d0.

📒 Files selected for processing (2)

• src/wallet/wallet.cpp
• test/functional/wallet_encryption.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

CWallet::ChangeWalletPassphrase now records the wallet's locked and mixing-only states. It restores the prior lock mode after changing the passphrase and uses mixing-only access during passphrase verification when the wallet was locked or mixing-only. A functional test covers passphrase changes in locked, mixing-only, and fully unlocked states, then checks signing and CoinJoin permissions.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c4305

The change keeps a mixing-only or locked wallet from becoming fully unlocked after a passphrase change, and a functional test covers the permission states. No merge-blocking risk is apparent.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: preserving wallet permissions during passphrase changes.
Description check		The description directly explains the wallet permission issue, the implemented fix, testing, and expected behavior.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

✅ Final review complete — no blockers (commit 6861fa5) · triage: low

[thepastaclaw]

Final validation — Phase 1 + Phase 2

At head 9652df1, the implementation correctly preserves successful passphrase-change permissions, retains existing incorrect-passphrase behavior and unlock timers, and adds behavioral regression coverage for all three wallet modes. No correctness blockers were found; one release-note nitpick remains under the project's explicit documentation guidance. This was a static verification: the supplied CI snapshot shows successful formatting and merge checks, but build-container jobs remain queued and completed source build/test results are not evidenced.

💬 1 nitpick(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained change to CWallet::ChangeWalletPassphrase that preserves existing unlock permissions and adds focused regression coverage without introducing large or intricate key-handling changes.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 94% left, weekly 95% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 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/wallet/wallet.cpp`:
- [NITPICK] src/wallet/wallet.cpp:580-595: User-visible post-change lock-state contract is undocumented (no release note)
  This fixes a user-visible wallet permission bug: a successful walletpassphrasechange no longer upgrades a mixing-only wallet to full signing/spending access. The Release notes section of doc/developer-notes.md explicitly calls for PR-specific notes for significant bug fixes and visible end-user changes, but this PR changes only the implementation and functional test. Add doc/release-notes-7829.md explaining that successful passphrase changes preserve the prior locked, mixing-only, or fully unlocked state without extending the existing unlock timeout.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At head 41310f4, the complete diff preserves successful passphrase-change permissions under the existing wallet locks without changing unlock deadlines, and adds behavioral regression coverage. The release note resolves the prior documentation finding; the shorter timer-test sleep does not establish an actionable defect. Validation was static only: build, lint, and main test checks were still queued in the supplied CI snapshot, and reported local results were not independently rerun.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained permission-preservation fix in CWallet::ChangeWalletPassphrase with focused regression coverage, rather than a large or intricate change to key handling.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 89% left, weekly 89% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

[thepastaclaw]

Re-review — Final validation — Phase 2 only (queue backlog)

Static verification of the complete diff at fc286ed found no correctness defects in the permission-preservation fix or its regression test. The prior release-note nitpick remains valid: the documentation described in the author’s reply is absent from this exact head. No builds or tests were run; the supplied CI snapshot shows successful formatting and preliminary checks, while container builds and title validation remain queued.

💬 1 nitpick(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained change to CWallet::ChangeWalletPassphrase to preserve existing unlock permissions, with direct regression coverage, and is not large or intricate despite touching key handling.
• Phase 1 reviewers: not run (skipped for throughput: 16 PRs queued, above the 10 limit)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 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/wallet/wallet.cpp`:
- [NITPICK] src/wallet/wallet.cpp:580-595: User-visible post-change lock-state contract is undocumented (no release note)
  (existing thread: https://github.com/dashpay/dash/pull/7829#discussion_r4205683930)
  The release note described in your reply is not included in fc286ed9439ad09948bb021bbd643731a4a8ce05: the complete base-to-head diff changes only wallet.cpp and wallet_encryption.py, and the head tree contains no doc/release-notes-7829.md. This fixes a user-visible permission escalation, so it falls under the release-note guidance in doc/developer-notes.md:1389–1397. Restore the note explaining that successful walletpassphrasechange preserves the prior locked, mixing-only, or fully unlocked mode without extending the existing unlock expiry.

[github-actions]

This pull request has conflicts, please rebase.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification of the complete diff at c4305d0 found no correctness or security defects in the permission-preservation fix or its regression coverage. The prior release-note finding remains valid: the documentation commit exists locally but is not reachable from this head, and no equivalent release-note entry is present. No builds or tests were run in this static lane; the supplied CI snapshot shows lint and formatting passing while source builds remain queued.

💬 1 nitpick(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained change to CWallet::ChangeWalletPassphrase that preserves existing unlock permissions using existing locking and cleanup helpers, with focused regression coverage, rather than a large or intricate key-handling change.
• Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 71% left, 5h 25% left
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 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/wallet/wallet.cpp`:
- [NITPICK] src/wallet/wallet.cpp:580-596: User-visible post-change lock-state contract is undocumented (no release note)
  (existing thread: https://github.com/dashpay/dash/pull/7829#discussion_r4205683930)
  The note described in your reply exists in commit 41310f4cd311d5e8d99d62da274db40150e0f9da, but that commit is not an ancestor of the current head. At c4305d07f2806ee51d559ccd53bd24fde247c556, the PR changes only wallet.cpp and wallet_encryption.py; doc/release-notes-7829.md is absent, and the current release-note files contain no equivalent entry. Because this fixes a user-visible wallet permission bug, it meets the release-note criteria in doc/developer-notes.md. Restore the note explaining that successful walletpassphrasechange preserves locked, mixing-only, or fully unlocked permissions without extending the existing unlock expiry.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at head 6861fa5: the fix preserves mixing-only permissions and relocks initially locked wallets before releasing the existing locks, with regression coverage for all three permission modes. No correctness defects were identified, but the previously added release note is absent from this head. This was static verification only; the supplied CI snapshot shows successful lint and several platform builds, while additional Linux builds and TSAN testing remain pending.

💬 1 nitpick(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained change to CWallet::ChangeWalletPassphrase that preserves existing unlock permissions and adds direct regression coverage, rather than a large or intricate key-handling change.
• Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 68% left, 5h 88% left
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 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/wallet/wallet.cpp`:
- [NITPICK] src/wallet/wallet.cpp:581-586: User-visible post-change lock-state contract is undocumented (no release note)
  (existing thread: https://github.com/dashpay/dash/pull/7829#discussion_r4205683930)
  The release note you added exists in commit 41310f4cd3, but doc/release-notes-7829.md is absent from the current head, and no equivalent entry appears in the current release notes. This fix changes user-visible wallet authorization: changing a mixing-only wallet's passphrase no longer grants signing or spending permissions. The Release notes section of doc/developer-notes.md calls for PR-specific notes for significant bug fixes and visible end-user changes. Restore the note documenting that successful walletpassphrasechange preserves the prior locked, mixing-only, or fully unlocked state without extending the existing unlock expiry.

[PastaPastaPasta]

approved