fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost)
Pull request description:
This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.
For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.
Both markers can still be parsed.
The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.
See discussion in #15740
ACKs for top commit:
achow101:
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
darosior:
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
This makes it easier to handle descriptor strings manually. E.g. an RPC call that takes an array of descriptors can now use '["desc": ".../0h/..."]'.
Both markers can still be parsed. The default for new descriptors is changed to h. In normalized form h is also used. For private keys the chosen marker is preserved in a round trip.
The hdkeypath field in getaddressinfo is also impacted by this change.
Since migration reloads the wallet, the wallet will always be locked
unless the passphrase is given. migratewallet can now take the
passphrase in order to unlock the wallet for migration.
5e65a216d1fd00c447757736d4f2899d235e731a wallet: Explicitly say migratewallet on encrypted wallets is unsupported (Andrew Chow)
88afc73ae0c67a4482ecd3d77eb2a8fd2673f82d tests: Test for migrating encrypted wallets (Andrew Chow)
86ef7b3c7be84e4183098f448c77ecc9ea7367ab wallet: Avoid null pointer deref when cleaning up migratewallet (Andrew Chow)
Pull request description:
When `migratewallet` fails, we do an automatic cleanup in order to reset everything so that the user does not experience any interruptions. However, this apparently has a segfault in it, caused by the the pointers to the watchonly and solvables wallets being nullptr. If those wallets are not created (either not needed, or failed early on), we will accidentally attempt to dereference these nullptrs, which causes a segfault.
This failure can be easily reached by trying to migrate an encrypted wallet. Currently, we can't migrate encrypted wallets because of how we unload wallets before migrating, and therefore forget the encryption key if the wallet was unlocked. So any encrypted wallets will fail, entering the cleanup, and because watchonly and solvables wallets don't exist yet, the segfault is reached.
This PR fixes this by not putting those nullptrs in a place that we will end up dereferencing them later. It also adds a test that uses the encrypted wallet issue.
ACKs for top commit:
S3RK:
reACK 5e65a216d1fd00c447757736d4f2899d235e731a
stickies-v:
ACK [5e65a21](5e65a216d1)
furszy:
diff ACK 5e65a21
Tree-SHA512: f75643797220d4232ad3ab8cb4b46d0f3667f00486e910ca748c9b6d174d446968f1ec4dd7f907da1be9566088849da7edcd8cd8f12de671c3241b513deb8e80
Due to an oversight, we cannot currently migrate encrypted wallets,
regardless of whether they are unlocked. Migrating such wallets will
trigger an error, and result in the cleanup being run. This conveniently
allows us to check some parts of the cleanup code.
Review note: The changes are complete, because self.options.descriptors
is set to None in parse_args (test_framework.py).
A value of None implies -disablewallet, see the previous commit.
So if a call to add_wallet_options is missing, it will lead to a test
failure when the wallet is compiled in.
