Skip to content

Commit

Permalink
Merge bitcoin#23093: Add ability to flush keypool and always flush wh…
Browse files Browse the repository at this point in the history
…en upgrading non-HD to HD

6531599 test: Add check that newkeypool flushes change addresses too (Samuel Dobson)
84fa19c Add release notes for keypool flush changes (Samuel Dobson)
f9603ee Add test for flushing keypool with newkeypool (Samuel Dobson)
6f6f7bb Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson)
2434b10 Fix outdated keypool size default (Samuel Dobson)
22cc797 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 6531599
  meshcollider:
    Added new commit 6531599 to avoid invalidating previous ACKs.
  instagibbs:
    ACK bitcoin@6531599

Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774
  • Loading branch information
laanwj authored and knst committed Feb 14, 2025
1 parent 7f5fa35 commit f50d445
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
8 changes: 8 additions & 0 deletions doc/release-notes-23093.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Notable changes
===============

Updated RPCs
------------

- a new RPC `newkeypool` has been added, which will flush (entirely
clear and refill) the keypool.
30 changes: 29 additions & 1 deletion src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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<CWallet> 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",
Expand Down Expand Up @@ -4642,6 +4669,7 @@ static const CRPCCommand commands[] =
{ "wallet", &listwallets, },
{ "wallet", &loadwallet, },
{ "wallet", &lockunspent, },
{ "wallet", &newkeypool, },
{ "wallet", &removeprunedfunds, },
{ "wallet", &rescanblockchain, },
{ "wallet", &send, },
Expand Down
14 changes: 14 additions & 0 deletions test/functional/wallet_keypool_hd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit f50d445

Please sign in to comment.