fix: reject mismatched HD chain wallet records

[thepastaclaw]

Issue being fixed or feature implemented

Malformed wallet database records can currently trip a process assertion while
loading HD chain state. A wallet database whose HD chain key type disagrees with
the serialized chain encryption flag is corrupt input and should be reported as
a wallet load error instead of aborting the process.

What was done?

Changed ReadKeyValue to validate HDCHAIN / CRYPTED_HDCHAIN records with a
normal conditional check. When the key type and serialized CHDChain state do
not match, wallet loading now returns a descriptive database error.

Added focused unit coverage for both mismatch directions.

How Has This Been Tested?

Ran:

git diff --check upstream/develop...HEAD
src/test/test_dash --run_test=walletdb_hdchain_tests
code-review dashpay/dash upstream/develop e85b88f74882b2ecf437cbce56441177e1955e11 "Reject mismatched HDCHAIN/CRYPTED_HDCHAIN wallet records gracefully instead of asserting during wallet load"

The pre-PR review gate returned ship.

Breaking Changes

None.

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)

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ff73eaa3-ce02-4b25-85db-a59a48019ea9

📥 Commits

Reviewing files that changed from the base of the PR and between ebfdb8b and e85b88f.

📒 Files selected for processing (2)

• src/wallet/test/walletdb_tests.cpp
• src/wallet/walletdb.cpp

---

Walkthrough

In walletdb.cpp, the ReadKeyValue function's handling of DBKeys::HDCHAIN and DBKeys::CRYPTED_HDCHAIN records changes from a debug-only assert (that CRYPTED_HDCHAIN matches chain.IsCrypted()) to a runtime check that sets strErr to "HD chain type mismatch" and returns false on mismatch. A new Boost test suite walletdb_hdchain_tests is added with a helper function and a regression test that verifies both key types produce this error when the crypted/non-crypted state mismatches.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: rejecting mismatched HD chain wallet records instead of asserting.
Description check	✅ Passed	The description is directly related to the code changes and test additions, and accurately explains the fix.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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]

✅ Review complete (commit 8fbae2b)

[thepastaclaw]

Code Review

Small, well-scoped fix that converts an assert over the HD chain record-type invariant into a graceful wallet load error. strType is the branch discriminator so it is set before the early return, and WalletBatch::IsKeyType() already covers HDCHAIN/CRYPTED_HDCHAIN so the loader surfaces DBErrors::CORRUPT instead of aborting. New unit tests exercise both mismatch directions; all four agents converged on zero findings.

Note: GitHub does not allow me to approve my own PR, so this is posted as a COMMENT while preserving the clean exact-SHA review.

[thepastaclaw]

Code Review

Single-commit fix replacing an aborting assert in ReadKeyValue with a graceful wallet-load error on HD chain type/payload mismatch. Verified: strType is HDCHAIN/CRYPTED_HDCHAIN at the return-false point, both are in IsKeyType(), so LoadWallet classifies the failure as DBErrors::CORRUPT — matching the surrounding HDPUBKEY corrupt pattern. Latest delta only consolidates the new tests into the existing walletdb_tests suite by promoting it to WalletTestingSetup, which does not affect the pre-existing stream-deserialization test. Prior review at e85b88f was clean and no findings carry forward (prior_findings_status: []).

Note: GitHub does not allow me to approve my own PR, so this is posted as a COMMENT while preserving the clean exact-SHA review.

[thepastaclaw]

CI scope check for the current red jobs on 8fbae2b3a8528eb0847c92c1d284341c9509cf46: these failures are unrelated to this PR's wallet DB change, so I did not modify, rebase, push, or rerun the branch.

This PR only changes:

src/wallet/test/walletdb_tests.cpp
src/wallet/walletdb.cpp

The linux64_tsan-test / Test source failure is in feature_asset_locks.py, after the test mines a new llmq_test_platform quorum and then observes a propagated asset-unlock transaction in the mempool while expecting size 0:

feature_asset_locks.py:435 -> check_mempool_result(... asset_unlock_tx_late ...)
feature_asset_locks.py:158 -> assert_equal(node.getmempoolinfo()['size'], self.mempool_size)
AssertionError: not(1 == 0)

That test is already tracked as intermittent in #7310; I added this run as new evidence there.

There is also a current linux64-test / Test source failure in the same Actions run, but it is a separate known flaky functional test: feature_dip3_v19.py timed out in wait_for_sporks_same() during v19 activation. That is tracked in #6702, and I added this run there too. feature_asset_locks.py only failed an early attempt in that job and then passed on retry.

The PR fix remains proper as-is: the branch touches only local wallet record loading/tests, while both red checks are pre-existing functional-test timing/sync flakes outside the PR diff.