From f50d445b6c0ed2e4c163895d669515eb96863d7a Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Thu, 14 Oct 2021 18:05:54 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#23093: Add ability to flush keypool and always flush when upgrading non-HD to HD 6531599f422524fbbcc43816121e7536cf79d66c test: Add check that newkeypool flushes change addresses too (Samuel Dobson) 84fa19c77a2c8d0d01add2daf18b42af07c17710 Add release notes for keypool flush changes (Samuel Dobson) f9603ee4e05d7f0bd7d81f5cf24168c1aec8e5b0 Add test for flushing keypool with newkeypool (Samuel Dobson) 6f6f7bb36c492fa76aeda6513be58ca822ea1968 Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson) 2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Fix outdated keypool size default (Samuel Dobson) 22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Add newkeypool RPC to flush the keypool (Samuel Dobson) Pull request description: This PR makes two main changes: 1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool. 2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys. This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call. ACKs for top commit: laanwj: re-ACK 6531599f422524fbbcc43816121e7536cf79d66c meshcollider: Added new commit 6531599f422524fbbcc43816121e7536cf79d66c to avoid invalidating previous ACKs. instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/23093/commits/6531599f422524fbbcc43816121e7536cf79d66c Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774 --- doc/release-notes-23093.md | 8 ++++++++ src/wallet/rpcwallet.cpp | 30 +++++++++++++++++++++++++++- test/functional/wallet_keypool_hd.py | 14 +++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 doc/release-notes-23093.md diff --git a/doc/release-notes-23093.md b/doc/release-notes-23093.md new file mode 100644 index 0000000000000..ce75de56d263c --- /dev/null +++ b/doc/release-notes-23093.md @@ -0,0 +1,8 @@ +Notable changes +=============== + +Updated RPCs +------------ + +- a new RPC `newkeypool` has been added, which will flush (entirely +clear and refill) the keypool. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 172bb82fefd57..e09a1a398c0fe 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1747,7 +1747,7 @@ static RPCHelpMan keypoolrefill() "\nFills the keypool."+ HELP_REQUIRING_PASSPHRASE, { - {"newsize", RPCArg::Type::NUM, RPCArg::Default{int{DEFAULT_KEYPOOL_SIZE}}, "The new keypool size"}, + {"newsize", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%u, or as set by -keypool", DEFAULT_KEYPOOL_SIZE)}, "The new keypool size"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -1786,6 +1786,33 @@ static RPCHelpMan keypoolrefill() } +static RPCHelpMan newkeypool() +{ + return RPCHelpMan{"newkeypool", + "\nEntirely clears and refills the keypool."+ + HELP_REQUIRING_PASSPHRASE, + {}, + RPCResult{RPCResult::Type::NONE, "", ""}, + RPCExamples{ + HelpExampleCli("newkeypool", "") + + HelpExampleRpc("newkeypool", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + + LOCK(pwallet->cs_wallet); + + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true); + spk_man.NewKeyPool(); + + return NullUniValue; +}, + }; +} + + static RPCHelpMan walletpassphrase() { return RPCHelpMan{"walletpassphrase", @@ -4642,6 +4669,7 @@ static const CRPCCommand commands[] = { "wallet", &listwallets, }, { "wallet", &loadwallet, }, { "wallet", &lockunspent, }, + { "wallet", &newkeypool, }, { "wallet", &removeprunedfunds, }, { "wallet", &rescanblockchain, }, { "wallet", &send, }, diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py index 7ef2e16401038..52e0e6009bcf1 100755 --- a/test/functional/wallet_keypool_hd.py +++ b/test/functional/wallet_keypool_hd.py @@ -161,6 +161,20 @@ def run_test(self): assert_equal(wi['keypoolsize_hd_internal'], 100) assert_equal(wi['keypoolsize'], 100) + if not self.options.descriptors: + # Check that newkeypool entirely flushes the keypool + start_keypath = nodes[0].getaddressinfo(nodes[0].getnewaddress())['hdkeypath'] + start_change_keypath = nodes[0].getaddressinfo(nodes[0].getrawchangeaddress())['hdkeypath'] + # flush keypool and get new addresses + nodes[0].newkeypool() + end_keypath = nodes[0].getaddressinfo(nodes[0].getnewaddress())['hdkeypath'] + end_change_keypath = nodes[0].getaddressinfo(nodes[0].getrawchangeaddress())['hdkeypath'] + # The new keypath index should be 100 more than the old one + new_index = int(start_keypath.rsplit('/', 1)[1][:-1]) + 100 + new_change_index = int(start_change_keypath.rsplit('/', 1)[1][:-1]) + 100 + assert_equal(end_keypath, "m/0'/0'/" + str(new_index) + "'") + assert_equal(end_change_keypath, "m/0'/1'/" + str(new_change_index) + "'") + # create a blank wallet nodes[0].createwallet(wallet_name='w2', blank=True, disable_private_keys=True) w2 = nodes[0].get_wallet_rpc('w2')