From 22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sun, 26 Sep 2021 11:36:59 +1300 Subject: [PATCH 1/6] Add newkeypool RPC to flush the keypool --- src/wallet/rpcwallet.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 77757e79a33..002029d81f4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1892,6 +1892,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", @@ -4773,6 +4800,7 @@ static const CRPCCommand commands[] = { "wallet", &listwallets, }, { "wallet", &loadwallet, }, { "wallet", &lockunspent, }, + { "wallet", &newkeypool, }, { "wallet", &removeprunedfunds, }, { "wallet", &rescanblockchain, }, { "wallet", &send, }, From 2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sun, 26 Sep 2021 11:37:53 +1300 Subject: [PATCH 2/6] Fix outdated keypool size default --- src/wallet/rpcwallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 002029d81f4..bc29da82ac4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1853,7 +1853,7 @@ static RPCHelpMan keypoolrefill() "\nFills the keypool."+ HELP_REQUIRING_PASSPHRASE, { - {"newsize", RPCArg::Type::NUM, RPCArg::Default{100}, "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{ From 6f6f7bb36c492fa76aeda6513be58ca822ea1968 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sun, 26 Sep 2021 12:00:35 +1300 Subject: [PATCH 3/6] Make legacy wallet upgrades from non-HD to HD always flush the keypool --- src/wallet/scriptpubkeyman.cpp | 2 +- test/functional/wallet_upgradewallet.py | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index fe41f9b8ccc..08403368317 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -489,7 +489,7 @@ bool LegacyScriptPubKeyMan::Upgrade(int prev_version, int new_version, bilingual } // Regenerate the keypool if upgraded to HD if (hd_upgrade) { - if (!TopUp()) { + if (!NewKeyPool()) { error = _("Unable to generate keys"); return false; } diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index ed98db55c9d..917293ea8c0 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -235,12 +235,7 @@ class UpgradeWalletTest(BitcoinTestFramework): seed_id = bytearray(seed_id) seed_id.reverse() old_kvs = new_kvs - # First 2 keys should still be non-HD - for i in range(0, 2): - info = wallet.getaddressinfo(wallet.getnewaddress()) - assert 'hdkeypath' not in info - assert 'hdseedid' not in info - # Next key should be HD + # New keys should be HD (the two old keys have been flushed) info = wallet.getaddressinfo(wallet.getnewaddress()) assert_equal(seed_id.hex(), info['hdseedid']) assert_equal('m/0\'/0\'/0\'', info['hdkeypath']) @@ -291,14 +286,7 @@ class UpgradeWalletTest(BitcoinTestFramework): hd_chain_version, external_counter, seed_id, internal_counter = struct.unpack(' Date: Sun, 26 Sep 2021 13:43:55 +1300 Subject: [PATCH 4/6] Add test for flushing keypool with newkeypool --- test/functional/wallet_keypool.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index c7149932342..627a7508509 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -138,6 +138,16 @@ class KeyPoolTest(BitcoinTestFramework): 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'] + nodes[0].newkeypool() + end_keypath = nodes[0].getaddressinfo(nodes[0].getnewaddress())['hdkeypath'] + # The new keypath index should be 100 more than the old one + keypath_prefix = start_keypath.rsplit('/', 1)[0] + new_index = int(start_keypath.rsplit('/', 1)[1][:-1]) + 100 + assert_equal(end_keypath, keypath_prefix + '/' + str(new_index) + '\'') + # create a blank wallet nodes[0].createwallet(wallet_name='w2', blank=True, disable_private_keys=True) w2 = nodes[0].get_wallet_rpc('w2') From 84fa19c77a2c8d0d01add2daf18b42af07c17710 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sun, 26 Sep 2021 00:07:14 +1200 Subject: [PATCH 5/6] Add release notes for keypool flush changes --- doc/release-notes-23093.md | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 00000000000..68fbaec53c5 --- /dev/null +++ b/doc/release-notes-23093.md @@ -0,0 +1,11 @@ +Notable changes +=============== + +Updated RPCs +------------ + +- `upgradewallet` will now automatically flush the keypool if upgrading +from a non-HD wallet to an HD wallet, to immediately start using the +newly-generated HD keys. +- a new RPC `newkeypool` has been added, which will flush (entirely +clear and refill) the keypool. From 6531599f422524fbbcc43816121e7536cf79d66c Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Thu, 14 Oct 2021 16:52:59 +1300 Subject: [PATCH 6/6] test: Add check that newkeypool flushes change addresses too --- test/functional/wallet_keypool.py | 8 ++++++-- test/functional/wallet_upgradewallet.py | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 627a7508509..79235646b09 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -141,12 +141,16 @@ class KeyPoolTest(BitcoinTestFramework): 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 - keypath_prefix = start_keypath.rsplit('/', 1)[0] new_index = int(start_keypath.rsplit('/', 1)[1][:-1]) + 100 - assert_equal(end_keypath, keypath_prefix + '/' + str(new_index) + '\'') + 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) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 917293ea8c0..5800880830d 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -234,13 +234,13 @@ class UpgradeWalletTest(BitcoinTestFramework): assert_equal(1, hd_chain_version) seed_id = bytearray(seed_id) seed_id.reverse() - old_kvs = new_kvs - # New keys should be HD (the two old keys have been flushed) + + # New keys (including change) should be HD (the two old keys have been flushed) info = wallet.getaddressinfo(wallet.getnewaddress()) assert_equal(seed_id.hex(), info['hdseedid']) assert_equal('m/0\'/0\'/0\'', info['hdkeypath']) prev_seed_id = info['hdseedid'] - # Change key should be the same keypool + # Change key should be HD and from the same keypool info = wallet.getaddressinfo(wallet.getrawchangeaddress()) assert_equal(prev_seed_id, info['hdseedid']) assert_equal('m/0\'/0\'/1\'', info['hdkeypath'])