Merge #8389: [0.13] Create a new HD seed after encrypting the wallet

de45c06 [Wallet] Add CKeyMetadata record for HDMasterKey(s), factor out HD key generation (Jonas Schnelli)
f142c11 [0.13] Create a new HD seed after encrypting the wallet (Jonas Schnelli)

doc/release-notes.md

@@ -135,6 +135,8 @@ Existing wallets will still use traditional key generation.
 Backups of HD wallets, regardless of when they have been created, can
 therefore be used to re-generate all possible private keys, even the
 ones which haven't already been generated during the time of the backup.
+**Attention:** Encrypting the wallet will create a new seed which requires
+a new backup!
 
 HD key generation for new wallets can be disabled by `-usehd=0`. Keep in
 mind that this flag only has affect on newly created wallets.

qa/rpc-tests/keypool.py

@@ -12,6 +12,11 @@ class KeyPoolTest(BitcoinTestFramework):
 
     def run_test(self):
         nodes = self.nodes
+        addr_before_encrypting = nodes[0].getnewaddress()
+        addr_before_encrypting_data = nodes[0].validateaddress(addr_before_encrypting)
+        wallet_info_old = nodes[0].getwalletinfo()
+        assert(addr_before_encrypting_data['hdmasterkeyid'] == wallet_info_old['hdmasterkeyid'])
+        
         # Encrypt wallet and wait to terminate
         nodes[0].encryptwallet('test')
         bitcoind_processes[0].wait()
@@ -19,6 +24,11 @@ class KeyPoolTest(BitcoinTestFramework):
         nodes[0] = start_node(0, self.options.tmpdir)
         # Keep creating keys
         addr = nodes[0].getnewaddress()
+        addr_data = nodes[0].validateaddress(addr)
+        wallet_info = nodes[0].getwalletinfo()
+        assert(addr_before_encrypting_data['hdmasterkeyid'] != wallet_info['hdmasterkeyid'])
+        assert(addr_data['hdmasterkeyid'] == wallet_info['hdmasterkeyid'])
+        
         try:
             addr = nodes[0].getnewaddress()
             raise AssertionError('Keypool should be exhausted after one address')

src/wallet/rpcwallet.cpp

@@ -2081,7 +2081,7 @@ UniValue encryptwallet(const UniValue& params, bool fHelp)
     // slack space in .dat files; that is bad if the old data is
     // unencrypted private keys. So:
     StartShutdown();
-    return "wallet encrypted; Bitcoin server stopping, restart to run with encrypted wallet. The keypool has been flushed, you need to make a new backup.";
+    return "wallet encrypted; Bitcoin server stopping, restart to run with encrypted wallet. The keypool has been flushed and a new HD seed was generated (if you are using HD). You need to make a new backup.";
 }
 
 UniValue lockunspent(const UniValue& params, bool fHelp)

src/wallet/wallet.cpp

@@ -626,6 +626,15 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)
 
         Lock();
         Unlock(strWalletPassphrase);
+
+        // if we are using HD, replace the HD master key (seed) with a new one
+        if (!hdChain.masterKeyID.IsNull()) {
+            CKey key;
+            CPubKey masterPubKey = GenerateNewHDMasterKey();
+            if (!SetHDMasterKey(masterPubKey))
+                return false;
+        }
+
         NewKeyPool();
         Lock();
 
@@ -1166,19 +1175,42 @@ CAmount CWallet::GetChange(const CTransaction& tx) const
     return nChange;
 }
 
-bool CWallet::SetHDMasterKey(const CKey& key)
+CPubKey CWallet::GenerateNewHDMasterKey()
 {
-    LOCK(cs_wallet);
+    CKey key;
+    key.MakeNewKey(true);
 
-    // ensure this wallet.dat can only be opened by clients supporting HD
+    int64_t nCreationTime = GetTime();
-    SetMinVersion(FEATURE_HD);
+    CKeyMetadata metadata(nCreationTime);
 
-    // store the key as normal "key"/"ckey" object
+    // calculate the pubkey
-    // in the database
-    // key metadata is not required
     CPubKey pubkey = key.GetPubKey();
+    assert(key.VerifyPubKey(pubkey));
+
+    // set the hd keypath to "m" -> Master, refers the masterkeyid to itself
+    metadata.hdKeypath     = "m";
+    metadata.hdMasterKeyID = pubkey.GetID();
+
+    {
+        LOCK(cs_wallet);
+
+        // mem store the metadata
+        mapKeyMetadata[pubkey.GetID()] = metadata;
+
+        // write the key&metadata to the database
         if (!AddKeyPubKey(key, pubkey))
             throw std::runtime_error("CWallet::GenerateNewKey(): AddKey failed");
+    }
+
+    return pubkey;
+}
+
+bool CWallet::SetHDMasterKey(const CPubKey& pubkey)
+{
+    LOCK(cs_wallet);
+
+    // ensure this wallet.dat can only be opened by clients supporting HD
+    SetMinVersion(FEATURE_HD);
 
     // store the keyid (hash160) together with
     // the child index counter in the database
@@ -3299,8 +3331,8 @@ bool CWallet::InitLoadWallet()
         if (GetBoolArg("-usehd", DEFAULT_USE_HD_WALLET) && walletInstance->hdChain.masterKeyID.IsNull()) {
             // generate a new master key
             CKey key;
-            key.MakeNewKey(true);
+            CPubKey masterPubKey = walletInstance->GenerateNewHDMasterKey();
-            if (!walletInstance->SetHDMasterKey(key))
+            if (!walletInstance->SetHDMasterKey(masterPubKey))
                 throw std::runtime_error("CWallet::GenerateNewKey(): Storing master key failed");
         }
         CPubKey newDefaultKey;

src/wallet/wallet.h

@@ -901,8 +901,11 @@ public:
     bool SetHDChain(const CHDChain& chain, bool memonly);
     const CHDChain& GetHDChain() { return hdChain; }
 
+    /* Generates a new HD master key (will not be activated) */
+    CPubKey GenerateNewHDMasterKey();
+    
     /* Set the current HD master key (will reset the chain child index counters) */
-    bool SetHDMasterKey(const CKey& key);
+    bool SetHDMasterKey(const CPubKey& key);
 };
 
 /** A key allocated from the key pool. */