Merge #11022: Basic keypool topup

d34957e [wallet] [tests] Add keypool topup functional test (Jonas Schnelli)
095142d [wallet] keypool mark-used and topup (John Newbery)
c25d90f [wallet] Add HasUnusedKeys() helper (John Newbery)
f2123e3 [wallet] Cache keyid -> keypool id mappings (John Newbery)
83f1ec3 [wallet] Don't hold cs_LastBlockFile while calling setBestChain (John Newbery)
2376bfc [wallet] [moveonly] Move LoadKeyPool to cpp (Matt Corallo)
cab8557 [wallet] [moveonly] Move CAffectedKeysVisitor (Jonas Schnelli)

Pull request description:

  This PR contains the first part of #10882 :

  - if a key from the keypool is used, mark all keys up to that key as used, and then try to top up the keypool
  - top up the keypool on startup

  Notably, it does not stop the node or prevent the best block from advancing if the keypool drops below a threshold (which means that transactions may be missed and funds lost if restoring from an old HD wallet backup).

Tree-SHA512: ac681fefeaf7ec2aab2fa1da93d12273ea80bd05eb48d7b3b551ea6e5d975dd97ba7de52b7fba52993823280ac4079cc36cf78a27dac708107ebf8fb6326142b

test/functional/keypool-topup.py

@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test HD Wallet keypool restore function.
+
+Two nodes. Node1 is under test. Node0 is providing transactions and generating blocks.
+
+- Start node1, shutdown and backup wallet.
+- Generate 110 keys (enough to drain the keypool). Store key 90 (in the initial keypool) and key 110 (beyond the initial keypool). Send funds to key 90 and key 110.
+- Stop node1, clear the datadir, move wallet file back into the datadir and restart node1.
+- connect node1 to node0. Verify that they sync and node1 receives its funds."""
+import shutil
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    connect_nodes_bi,
+    sync_blocks,
+)
+
+class KeypoolRestoreTest(BitcoinTestFramework):
+    def __init__(self):
+        super().__init__()
+        self.setup_clean_chain = True
+        self.num_nodes = 2
+        self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolmin=20']]
+
+    def run_test(self):
+        self.tmpdir = self.options.tmpdir
+        self.nodes[0].generate(101)
+
+        self.log.info("Make backup of wallet")
+
+        self.stop_node(1)
+
+        shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak")
+        self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1])
+        connect_nodes_bi(self.nodes, 0, 1)
+
+        self.log.info("Generate keys for wallet")
+
+        for _ in range(90):
+            addr_oldpool = self.nodes[1].getnewaddress()
+        for _ in range(20):
+            addr_extpool = self.nodes[1].getnewaddress()
+
+        self.log.info("Send funds to wallet")
+
+        self.nodes[0].sendtoaddress(addr_oldpool, 10)
+        self.nodes[0].generate(1)
+        self.nodes[0].sendtoaddress(addr_extpool, 5)
+        self.nodes[0].generate(1)
+        sync_blocks(self.nodes)
+
+        self.log.info("Restart node with wallet backup")
+
+        self.stop_node(1)
+
+        shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat")
+
+        self.log.info("Verify keypool is restored and balance is correct")
+
+        self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1])
+        connect_nodes_bi(self.nodes, 0, 1)
+        self.sync_all()
+
+        assert_equal(self.nodes[1].getbalance(), 15)
+        assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive")
+
+        # Check that we have marked all keys up to the used keypool key as used
+        assert_equal(self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/111'")
+
+if __name__ == '__main__':
+    KeypoolRestoreTest().main()

test/functional/test_runner.py

@@ -79,6 +79,7 @@ BASE_SCRIPTS= [
     'rawtransactions.py',
     'reindex.py',
     # vv Tests less than 30s vv
+    'keypool-topup.py',
     'zmq_test.py',
     'mempool_resurrect_test.py',
     'txn_doublespend.py --mineblock',

test/functional/wallet-hd.py

@@ -9,7 +9,6 @@ from test_framework.util import (
     assert_equal,
     connect_nodes_bi,
 )
-import os
 import shutil
 
 
@@ -72,10 +71,12 @@ class WalletHDTest(BitcoinTestFramework):
 
         self.log.info("Restore backup ...")
         self.stop_node(1)
-        os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat")
+        # we need to delete the complete regtest directory
+        # otherwise node1 would auto-recover all funds in flag the keypool keys as used
+        shutil.rmtree(tmpdir + "/node1/regtest/blocks")
+        shutil.rmtree(tmpdir + "/node1/regtest/chainstate")
         shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat")
         self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1])
-        #connect_nodes_bi(self.nodes, 0, 1)
 
         # Assert that derivation is deterministic
         hd_add_2 = None
@@ -85,11 +86,12 @@ class WalletHDTest(BitcoinTestFramework):
             assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(_+1)+"'")
             assert_equal(hd_info_2["hdmasterkeyid"], masterkeyid)
         assert_equal(hd_add, hd_add_2)
+        connect_nodes_bi(self.nodes, 0, 1)
+        self.sync_all()
 
         # Needs rescan
         self.stop_node(1)
         self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1] + ['-rescan'])
-        #connect_nodes_bi(self.nodes, 0, 1)
         assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1)
 
         # send a tx and make sure its using the internal chain for the changeoutput