[rfc] Remove encrypting wallets that already exist #36027

Encrypting wallets after they have already been used seems like it is no longer a useful feature. There are a number of possible footguns with doing so:

• encrypting generates an entirely new set of descriptors which necessitates a new backup
• in generating the new descriptors, if the user had imported custom descriptors as active, those descriptors become inactive, which may lead to funds being sent to incorrect descriptors
• once the wallet is encrypted, existing funds are not moved, so any compromise of an unencrypted backup can still steal those funds

It has been possible to make born encrypted wallets for several years now. Given the above, I think we should make born encrypted wallets the only way to make an encrypted wallet, and stop allowing users to encrypt wallets after the fact.

Removing this also makes many of the atomicity issues that #35752 tries to address irrelevant.

For historical context, when encryption was introduced, there could only ever have been a single wallet loaded at a time and wallets were created by default, prefilled with keys. The only way to have a wallet was to encrypt it after the fact.

I'm in favor of this direction: simplify the code, simplify wallet operations (including backups), and reduce the chances of users making mistakes.

Overall in favor of this.

Given the above, I think we should make born encrypted wallets the only way to make an encrypted wallet, and stop allowing users to encrypt wallets after the fact.

Should we also consider disallowing the creation of unencrypted wallets that contain private keys? Are there any legitimate use cases for them?

Should we also consider disallowing the creation of unencrypted wallets that contain private keys? Are there any legitimate use cases for them?

I think we should still allow unencrypted wallets.

IIRC encryption has been optional because, depending on the user, losing the passphrase could be an equal or greater risk than hostile access to the wallet file.

Makes complete sense. A concrete way this bites users today:

1. User sets up inheritance, sharing keys and descriptors with heirs.
2. User encrypts the wallet. New active descriptors are generated from a new seed, and the shared descriptor is deactivated.
3. The inheritance setup is now stale and has to be redone. Nothing tells the user this.

Concept ACK

This presents both footguns for users, and footguns for developers, with very little actual utility. This would also resolve the oldest open issue: https://github.com/bitcoin/bitcoin/issues/3314