#36387 wallet: rpc: Deprecate `encryptwallet` RPC for encrypting existing wallets.

ReviewabilityReviewsAgreementSize
#36387 davidgumberg wallet: rpc: Deprecate `encryptwallet` RPC for encrypting existing wallets.Waiting on author0Strong16 + 134 tests
  • Deprecate encrypting existing wallets to prevent fund loss and backup invalidation
  • Avoid leaving unencrypted keys on disk or unexpectedly altering spending conditions on users
  • Ready to review
  • Author response to documentation notes on GUI encryption scope is pending
  • Concept approval noting creating new encrypted wallets and sweeping is cleaner (pablomartin4btc)
  • Nonblocking suggestion to clarify GUI deprecation scope in release notes (pablomartin4btc)
  • Concept approval without stated reasons (pseudoramdom, w0xlt)
  • 16 lines added or modified outside tests
  • 134 lines added or modified in tests
  • 73 lines removed in total
  • 18 files, 8 commits
Wallet#13 of 92
P2 · fund safety
  • P2 because encrypting active wallets risks fund loss by invalidating prior backups unexpectedly
  • Unencrypted key material can remain on storage and imported descriptor conditions can silently mutate
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Deprecates the `encryptwallet` RPC method, requiring nodes to be started with `-deprecatedrpc=encryptwallet` to use it. Refactors the functional test suite to create encrypted wallets upfront via `createwallet` with a passphrase instead of encrypting existing wallets, while gating tests of `encryptwallet` behind the deprecation flag.

Problem: Encrypting a wallet after creation has severe footguns: unencrypted key material often remains on persistent storage, regenerating keys upon encryption invalidates earlier backups (risking fund loss), and encrypting wallets containing manually imported descriptors can change spending conditions unexpectedly (#26607).

Evidence of importance:

  • RFC issue #36027: '[rfc] Remove encrypting wallets that already exist'
  • PR description citing #26607: 'Encrypting a wallet that has manually imported descriptors will cause the wallet to change the spending conditions out from under the user.'

Scope: The C++ change in `src/wallet/rpc/encrypt.cpp` is only 5 lines; the majority of the diff consists of test framework helper updates and test refactorings to avoid post-creation encryption.

Risk of a mistake: medium (wallet, rpc_api). Deprecating an existing RPC breaks external scripts or user workflows that encrypt wallets after creation unless the deprecation flag is supplied.

Reviewability

Paused · Waiting on author

Open objection from pablomartin4btc with no author reply for 11 days; review would repeat what has been asked.

Waiting on (from the thread read):

  • author: response to review comments and note regarding GUI deprecation 2026-10-21

Agreement

Strong

substantive support, no open objection (pablomartin4btc)

Thread notes: pablomartin4btc provided a substantive Concept ACK along with code review nits and suggested addressing the GUI Encrypt Wallet status in documentation.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
pablomartin4btc (member)suggestion, scopeMention in the description and release notes that the GUI Encrypt Wallet action is unaffected, or deprecate GUI and RPC togetheropenansweredno2026-10-21: 'The GUI "Encrypt Wallet…" action (`WalletView::encryptWallet` → `interfaces::Wallet::encryptWallet`) is unaffected and is probably where most users hit this. It might be worth mentioning in the description and release notes so the GUI and RPC deprecations ship in the same release (?).'

Support:

  • pseudoramdom (member) (Concept ACK, 2026-09-30): (no reason given) [not substantive]
  • w0xlt (contributor) (Concept ACK, 2026-09-30): (no reason given) [not substantive]
  • pablomartin4btc (member) (Concept ACK, 2026-10-21) [both: doc/release-notes-deprecateencryptwallet.md, test/functional/test_framework/test_framework.py, test/functional/wallet_gethdkeys.py, test/functional/wallet_encryption.py, test/functional/wallet_listdescriptors.py, test/functional/wallet_bumpfee.py, test/functional/rpc_deprecated.py]: creating today a new encrypted wallet and sendall-ing to it is equivalent and cleaner

Participants:

  • pseudoramdom (member, support, 1 statements 2026-09-30 to 2026-09-30): Concept ACK
  • w0xlt (contributor, support, 1 statements 2026-09-30 to 2026-09-30): Concept ACK
  • pablomartin4btc (member, support, 11 statements 2026-10-20 to 2026-10-21): Concept ACK with review nits and suggestion on GUI encrypt wallet scope

Files

207 lines under test/bench/ci.

  • test/functional/wallet_bumpfee.py +22/-19
  • test/functional/rpc_psbt.py +31/-9
  • test/functional/wallet_listdescriptors.py +22/-9
  • test/functional/wallet_multiwallet.py +12/-8
  • test/functional/test_framework/test_framework.py +14/-5
  • test/functional/interface_bitcoin_cli.py +7/-8
  • test/functional/wallet_signrawtransactionwithwallet.py +6/-7
  • doc/release-notes-deprecateencryptwallet.md +11/-0
  • test/functional/wallet_fundrawtransaction.py +4/-7
  • test/functional/rpc_deprecated.py +6/-0
  • src/wallet/rpc/encrypt.cpp +5/-0
  • test/functional/wallet_blank.py +2/-0
  • test/functional/wallet_descriptor.py +1/-1
  • test/functional/wallet_encryption.py +2/-0
  • test/functional/wallet_gethdkeys.py +2/-0
  • test/functional/wallet_createwallet.py +1/-0
  • test/functional/wallet_derivehdkey.py +1/-0
  • test/functional/wallet_keypool.py +1/-0

Card

The short text the ranking pass compares across PRs.

Deprecates the encryptwallet RPC method for encrypting existing unencrypted wallets, requiring -deprecatedrpc=encryptwallet ahead of planned removal. Post-creation encryption carries footguns: unencrypted key material may linger on disk, new keys generated after encryption invalidate earlier backups, and encrypting wallets with imported descriptors can alter spending conditions unexpectedly (#26607). Creating encrypted wallets from inception using createwallet remains fully supported. Functional tests are refactored to set up encrypted wallets at creation time rather than encrypting existing ones.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:15, confidence high, input hash da9f89ca0be578b0
thread read through 2026-10-21 (13 statements), head 2cc0946985
labels: Wallet

Categories

Wallet

P2 · fund safety

Deprecating `encryptwallet` addresses a major fund-safety footgun where encrypting existing wallets invalidates prior backups and alters spending conditions unexpectedly. As noted in RFC #36027 and issue #26607, encrypting an active wallet leaves unencrypted material on disk and changes spending conditions out from under the user, making deprecation strategically important for fund safety.

Membership: Labeled Wallet and modifies wallet encryption RPC in src/wallet/rpc/encrypt.cpp.

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