.
. #35352
pull TianYuBaBg wants to merge 1 commits into bitcoin:master from TianYuBaBg:fix/encryptwallet-assert-replacement changing 1 files +6 −2-
TianYuBaBg commented at 8:29 PM on May 21, 2026: none
-
dd0e43ec1f
wallet: Replace assert(false) with CHECK_NONFATAL(false) in EncryptWallet
In CWallet::EncryptWallet(), two assert(false) calls serve as fatal error handlers for unrecoverable states -- when an SPK manager's Encrypt() fails mid-loop, and when the database TxnCommit() fails. assert() is semantically a debug aid. Replace it with the project's existing CHECK_NONFATAL(false) pattern, which: - In production: throws NonFatalCheckError, caught by the RPC server and returned as RPC_MISC_ERROR to the caller with a clear message. - In debug/fuzz builds (ABORT_ON_FAILED_ASSUME): still aborts, preserving the original crash-for-testing behavior. Additionally, clean up the master key entry that was optimistically added before the loop, so that HasEncryptionKeys() does not falsely report the wallet as encrypted after the failure.
- DrahtBot added the label Wallet on May 21, 2026
-
DrahtBot commented at 8:30 PM on May 21, 2026: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35352.
<!--021abf342d371248e50ceaed478a90ca-->
Reviews
See the guideline for information on the review process. A summary of reviews will appear here.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
-
sedited commented at 8:40 PM on May 21, 2026: contributor
That description gives me the impression that you are not familiar with conventions in this codebase, so I'm going to close this.
- sedited closed this on May 21, 2026
-
TianYuBaBg commented at 8:45 PM on May 21, 2026: none
Thanks for the feedback. I've rewritten the description to match the project's conventions.
- TianYuBaBg deleted the branch on May 21, 2026
-
achow101 commented at 8:49 PM on May 21, 2026: member
The comments above the asserts are very clear as to why they are there. Do not remove those asserts, we intentionally want to crash.
- fanquake renamed this:
wallet: Replace assert(false) with CHECK_NONFATAL(false) in EncryptWallet
.
on May 22, 2026 - DrahtBot removed the label Wallet on May 22, 2026
- bitcoin locked this on May 22, 2026
Contributors