#35966 wallet, rpc: log instead of returning false

full analysis

https://github.com/bitcoin/bitcoin/pull/35966 · vicjuma · +1/-1 in 1 files, 1 commits · labels: none

Goal

  • Prevent reporting wallet encryption failure when encryption succeeded but immediate unlock failed

This PR replaces returning `false` with a log message when `Unlock()` fails inside `CWallet::EncryptWallet`. The goal is to avoid reporting that encryption failed when the wallet had already been encrypted on disk before the unlock step.

Problem: If unlocking the wallet fails right after encryption, `EncryptWallet()` returns `false`, causing the RPC to report an encryption failure even though the wallet is already encrypted on disk.

Category: Wallet (#75 of 84)

P4 · cleanup

  • P4 because it addresses an extremely narrow edge case of an unlock failure right after encryption
  • Nuances of the RPC return value are marginal compared to safely handling wallet corruption

P4 because the PR targets an extremely narrow edge case of an impossible or catastrophic state during wallet encryption. If `Unlock` fails immediately after encrypting with that same passphrase, the wallet is in a corrupted state, making the nuance of the RPC error message marginal compared to safely handling the corruption.

Membership: Changes CWallet::EncryptWallet in src/wallet/wallet.cpp.

Factors: security/stability 0, bug 1, performance 0, user value 0, leverage 0

Reviewability: Paused: Waiting on author

  • Wait for author to update the PR with a different failure-handling mechanism
  • Wallet maintainer rejected continuing execution after a failed unlock as unsafe

The single-line diff was rejected by the wallet maintainer because continuing execution after failed unlock is unsafe. Further review is not worthwhile until the author updates the PR with a different failure-handling mechanism.

Author status: silent since 2026-08-17 after conceding that logging may not be the right fix

Open concerns:

  • achow101 objected that failing to unlock immediately after encrypting with the same passphrase indicates catastrophic failure or corruption, meaning execution should halt rather than merely log and proceed with wallet setup.

Resolved concerns:

  • katesalazar questioned if encryption of new wallets was broken; author clarified that encryption succeeded and only the subsequent unlock returned false.

Agreement: Disputed

  • Disputed approach to handling unlock failures after encryption
  • Unaddressed objection: failing to unlock indicates catastrophic corruption and should halt (achow101)
  • Author conceded that logging instead of halting may not be the right fix (vicjuma)
  • Questioned whether encryption of new wallets was broken (katesalazar)

Disputed: achow101 argues failing to unlock after encryption is catastrophic and logging is incorrect; author concurred logging may not be right.

achow101 stated that failing to unlock a freshly encrypted wallet means the wallet is corrupted and should not continue to be used or setup. The author acknowledged that logging might not be the proper response, leaving the PR in dispute.

  • achow101: 'If a wallet is successfully encrypted, and then we are unable to unlock it, that is a catastrophic failure... Logging a warning is not the right thing to do.'
  • vicjuma: 'Logging may not be right, this I did just to show that there might be an intermediate return value...'

Review verdicts (DrahtBot): 0

Files

0 lines under test/bench/ci.

  • src/wallet/wallet.cpp +1/-1

Card

This PR modifies wallet encryption so that a failure during the unlock phase logs a warning rather than returning false and reporting an encryption failure. It aims to clarify the RPC error message when a wallet has already been encrypted on disk but cannot be unlocked. However, the wallet maintainer pointed out that failing to unlock right after encryption is a catastrophic failure where continuing execution is unsafe. The PR is paused awaiting the author to rework the error-handling approach.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:56, confidence high, input hash abad21c6c925b635