fix: logic error in CheckDecryptionKey

[UdjinM6]

Issue being fixed or feature implemented

The issue was introduced via 392b51b

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

A conditional logic change was made in the CheckDecryptionKey function within the scriptpubkey manager. The condition that controls whether the HD chain seed is decrypted was inverted: previously, seed decryption occurred when the chain existed and was not encrypted; now it occurs when the chain exists and is encrypted. This alters the execution path to skip decryption for plaintext HD chains and perform decryption exclusively for encrypted chains.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Key consideration: The inversion of the encryption condition check requires careful verification that this is intentional and aligns with the intended wallet decryption behavior.
• Context needed: Understanding why the logic was inverted and confirming there are no unintended side effects on wallet initialization or decryption flows for both encrypted and unencrypted key chains.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies a logic error fix in the CheckDecryptionKey function, which directly corresponds to the main change described in the raw summary.
Description check	✅ Passed	The description references the specific commit that introduced the issue and mentions the function being fixed, demonstrating relevance to the changeset despite incomplete template sections.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b1b2c6 and d89646d.

📒 Files selected for processing (1)

• src/wallet/scriptpubkeyman.cpp (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/wallet/scriptpubkeyman.cpp

🧠 Learnings (1)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: mac-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: Lint / Run linters

🔇 Additional comments (1)

src/wallet/scriptpubkeyman.cpp (1)

225-231: Critical logic fix correctly applied.

This fix properly inverts the condition to decrypt only when the HD chain is encrypted. The previous logic would have attempted to decrypt plaintext chains (causing failure at line 550-551 in DecryptHDChain) and skipped decryption verification for encrypted chains, defeating the purpose of CheckDecryptionKey.

---

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 and usage tips.}

[PastaPastaPasta]

utACK d89646d

[knst]

utACK d89646d

[DashCoreAutoGuix]

Guix Automation has began to build this PR tagged as v23.0.0-devpr6938.d89646d5. A new comment will be made when the image is pushed.

[DashCoreAutoGuix]

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6938.d89646d5. The image should be on dockerhub soon.

[knst]

so far as #6932 is already merged; consider updating release notes here