Merge dashpay#6570: feat: mnemonic support for descriptor wallets

42e80f1 refactor: add prefix crypted_ for arguments related to crypted mnemonic (Konstantin Akimov)
c89e181 feat: improve wording for errors, RPC and logs (Konstantin Akimov)
b3a7475 refactor: removed dead commented code from serialize.h (Konstantin Akimov)
c8a6f69 refactor: use args instead gArgs in new mnemonic code (Konstantin Akimov)
d093506 fix: compilation errors due to advancing develop branch (Konstantin Akimov)
e636d19 test: multiple fixes in wallet_mnemonics to fix review comments (Konstantin Akimov)
3a8ab80 fix: intermittent error in wallet_mnemonic.py due to different order of descriptors (Konstantin Akimov)
4bd214e test: adjust commentary, ensure cb private key is "inactive" descriptor (Konstantin Akimov)
8eb5d4a fix: do not reset seed / mnemonic for descriptor wallets in case of encryption (Konstantin Akimov)
e27f95c fix: text message for HD wallets (both descriptors and non-descriptors) (Konstantin Akimov)
4f6be3d test: make functional test wallet_mnemonicbits.py works for descriptor wallets (Konstantin Akimov)
979ed95 feat: implement mnemonic for descriptor wallets (Konstantin Akimov)
267693f fix: expand text commentary for RPC encryptwallet (Konstantin Akimov)
d287f09 refactor: drop -usehd=1 from some functional tests so far as it is default option (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Descriptor wallets doesn't support yet mnemonics (BIP39), which are supported only by legacy (non-descriptor) wallet.

  ## What was done?
  This PR adds basic support BIP39 to descriptor wallets:
   - newly created descriptor wallets will have mnemonic
   - RPC `upgradetohd` now support descriptor wallets and let to set specifict mnemonic with passphrase to newly created wallet
   - mnemonic is shown when calling RPC `listdescriptors` if exists

  ## How Has This Been Tested?
  Functional test `wallet_mnemonicbits.py` support `--descriptors` now
  Functional test `wallet_upgradetohd.py` is updated also

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 42e80f1
  kwvg:
    utACK 42e80f1

Tree-SHA512: 2f55dd291356da8bfca31bd270c1a22056e7d2977e5f91761970cfaa6ccf5038cc930d59b6a572a2fcc6198ac9031ab177ca86d63155a6b3bd86bd92d3dbb4ee

Author: PastaPastaPasta

src/serialize.h

@@ -25,6 +25,7 @@
 #include <utility>
 #include <vector>
 
+#include <support/allocators/secure.h>
 #include <prevector.h>
 #include <span.h>
 
@@ -816,8 +817,8 @@ struct VectorFormatter
 /**
  *  string
  */
-template<typename Stream, typename C> void Serialize(Stream& os, const std::basic_string<C>& str);
-template<typename Stream, typename C> void Unserialize(Stream& is, std::basic_string<C>& str);
+template<typename Stream, typename A, typename B, typename C> void Serialize(Stream& os, const std::basic_string<A, B, C>& str);
+template<typename Stream, typename A, typename B, typename C> void Unserialize(Stream& is, std::basic_string<A, B, C>& str);
 
 /**
  * prevector
@@ -947,16 +948,16 @@ struct DefaultFormatter
 /**
  * string
  */
-template<typename Stream, typename C>
-void Serialize(Stream& os, const std::basic_string<C>& str)
+template<typename Stream, typename A, typename B, typename C>
+void Serialize(Stream& os, const std::basic_string<A, B, C>& str)
 {
     WriteCompactSize(os, str.size());
     if (!str.empty())
         os.write(MakeByteSpan(str));
 }
 
-template<typename Stream, typename C>
-void Unserialize(Stream& is, std::basic_string<C>& str)
+template<typename Stream, typename A, typename B, typename C>
+void Unserialize(Stream& is, std::basic_string<A, B, C>& str)
 {
     unsigned int nSize = ReadCompactSize(is);
     str.resize(nSize);

src/wallet/rpcdump.cpp

@@ -860,9 +860,9 @@ RPCHelpMan dumphdinfo()
         RPCResult{
                 RPCResult::Type::OBJ, "", "",
                 {
-                        {RPCResult::Type::STR_HEX, "hdseed", "The HD seed (bip32, in hex)"},
-                        {RPCResult::Type::STR, "mnemonic", "The mnemonic for this HD wallet (bip39, english words)"},
-                        {RPCResult::Type::STR, "mnemonicpassphrase", "The mnemonic passphrase for this HD wallet (bip39)"},
+                        {RPCResult::Type::STR_HEX, "hdseed", "The HD seed (BIP32, in hex)"},
+                        {RPCResult::Type::STR, "mnemonic", "The mnemonic for this HD wallet (BIP39, english words)"},
+                        {RPCResult::Type::STR, "mnemonicpassphrase", "The mnemonic passphrase for this HD wallet (BIP39)"},
                 }
         },
         RPCExamples{
@@ -1951,6 +1951,8 @@ RPCHelpMan listdescriptors()
             {
                 {RPCResult::Type::OBJ, "", "", {
                     {RPCResult::Type::STR, "desc", "Descriptor string representation"},
+                    {RPCResult::Type::STR, "mnemonic", "The mnemonic for this descriptor wallet (BIP39, english words). Presented only if private=true and created with a mnemonic"},
+                    {RPCResult::Type::STR, "mnemonicpassphrase", "The mnemonic passphrase for this descriptor wallet (BIP39). Presented only if private=true and created with a mnemonic"},
                     {RPCResult::Type::NUM, "timestamp", "The creation time of the descriptor"},
                     {RPCResult::Type::BOOL, "active", "Whether this descriptor is currently used to generate new addresses"},
                     {RPCResult::Type::BOOL, "internal", /*optional=*/true, "True if this descriptor is used to generate change addresses. False if this descriptor is used to generate receiving addresses; defined only for active descriptors"},
@@ -1997,6 +1999,14 @@ RPCHelpMan listdescriptors()
         if (!desc_spk_man->GetDescriptorString(descriptor, priv)) {
             throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string.");
         }
+        if (priv) {
+            SecureString mnemonic;
+            SecureString mnemonic_passphrase;
+            if (desc_spk_man->GetMnemonicString(mnemonic, mnemonic_passphrase) && !mnemonic.empty()) {
+                spk.pushKV("mnemonic", mnemonic.c_str());
+                spk.pushKV("mnemonicpassphrase", mnemonic_passphrase.c_str());
+            }
+        }
         spk.pushKV("desc", descriptor);
         spk.pushKV("timestamp", wallet_descriptor.creation_time);
         const bool active = active_spk_mans.count(desc_spk_man) != 0;

src/wallet/scriptpubkeyman.cpp

@@ -13,6 +13,7 @@
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <util/translation.h>
+#include <wallet/bip39.h>
 #include <wallet/scriptpubkeyman.h>
 
 bool LegacyScriptPubKeyMan::GetNewDestination(CTxDestination& dest, bilingual_str& error)
@@ -1844,6 +1845,7 @@ bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master
             keyFail = true;
             break;
         }
+        // TODO: test for mnemonics
         keyPass = true;
         if (m_decryption_thoroughly_checked)
             break;
@@ -1870,15 +1872,34 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
     {
         const CKey &key = key_in.second;
         CPubKey pubkey = key.GetPubKey();
+        assert(pubkey.GetID() == key_in.first);
+        const auto mnemonic_in = m_mnemonics.find(key_in.first);
         CKeyingMaterial secret(key.begin(), key.end());
         std::vector<unsigned char> crypted_secret;
         if (!EncryptSecret(master_key, secret, pubkey.GetHash(), crypted_secret)) {
             return false;
         }
+        std::vector<unsigned char> crypted_mnemonic;
+        std::vector<unsigned char> crypted_mnemonic_passphrase;
+        if (mnemonic_in != m_mnemonics.end()) {
+            const Mnemonic mnemonic = mnemonic_in->second;
+
+            CKeyingMaterial mnemonic_secret(mnemonic.first.begin(), mnemonic.first.end());
+            CKeyingMaterial mnemonic_passphrase_secret(mnemonic.second.begin(), mnemonic.second.end());
+            if (!EncryptSecret(master_key, mnemonic_secret, pubkey.GetHash(), crypted_mnemonic)) {
+                return false;
+            }
+            if (!EncryptSecret(master_key, mnemonic_passphrase_secret, pubkey.GetHash(), crypted_mnemonic_passphrase)) {
+                return false;
+            }
+        }
+
         m_map_crypted_keys[pubkey.GetID()] = make_pair(pubkey, crypted_secret);
-        batch->WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret);
+        m_crypted_mnemonics[pubkey.GetID()] = make_pair(crypted_mnemonic, crypted_mnemonic_passphrase);
+        batch->WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret, crypted_mnemonic, crypted_mnemonic_passphrase);
     }
     m_map_keys.clear();
+    m_mnemonics.clear();
     return true;
 }
 
@@ -2003,12 +2024,12 @@ void DescriptorScriptPubKeyMan::AddDescriptorKey(const CKey& key, const CPubKey
 {
     LOCK(cs_desc_man);
     WalletBatch batch(m_storage.GetDatabase());
-    if (!AddDescriptorKeyWithDB(batch, key, pubkey)) {
+    if (!AddDescriptorKeyWithDB(batch, key, pubkey, "", "")) {
         throw std::runtime_error(std::string(__func__) + ": writing descriptor private key failed");
     }
 }
 
-bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey)
+bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey, const SecureString& mnemonic, const SecureString& mnemonic_passphrase)
 {
     AssertLockHeld(cs_desc_man);
     assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
@@ -2025,22 +2046,37 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
         }
 
         std::vector<unsigned char> crypted_secret;
+        std::vector<unsigned char> crypted_mnemonic;
+        std::vector<unsigned char> crypted_mnemonic_passphrase;
         CKeyingMaterial secret(key.begin(), key.end());
+        CKeyingMaterial mnemonic_secret(mnemonic.begin(), mnemonic.end());
+        CKeyingMaterial mnemonic_passphrase_secret(mnemonic_passphrase.begin(), mnemonic_passphrase.end());
         if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) {
-                return EncryptSecret(encryption_key, secret, pubkey.GetHash(), crypted_secret);
+                if (!EncryptSecret(encryption_key, secret, pubkey.GetHash(), crypted_secret)) return false;
+                if (!mnemonic.empty()) {
+                    if (!EncryptSecret(encryption_key, mnemonic_secret, pubkey.GetHash(), crypted_mnemonic)) {
+                        return false;
+                    }
+                    if (!EncryptSecret(encryption_key, mnemonic_passphrase_secret, pubkey.GetHash(), crypted_mnemonic_passphrase)) {
+                        return false;
+                    }
+                }
+                return true;
             })) {
             return false;
         }
 
         m_map_crypted_keys[pubkey.GetID()] = make_pair(pubkey, crypted_secret);
-        return batch.WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret);
+        m_crypted_mnemonics[pubkey.GetID()] = make_pair(crypted_mnemonic, crypted_mnemonic_passphrase);
+        return batch.WriteCryptedDescriptorKey(GetID(), pubkey, crypted_secret, crypted_mnemonic, crypted_mnemonic_passphrase);
     } else {
         m_map_keys[pubkey.GetID()] = key;
-        return batch.WriteDescriptorKey(GetID(), pubkey, key.GetPrivKey());
+        m_mnemonics[pubkey.GetID()] = make_pair(mnemonic, mnemonic_passphrase);
+        return batch.WriteDescriptorKey(GetID(), pubkey, key.GetPrivKey(), mnemonic, mnemonic_passphrase);
     }
 }
 
-bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, bool internal)
+bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, bool internal)
 {
     LOCK(cs_desc_man);
     assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
@@ -2050,6 +2086,16 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
         return false;
     }
 
+    if (!secure_mnemonic.empty()) {
+        // TODO: remove duplicated code with AddKey()
+        SecureVector seed_key_tmp;
+        CMnemonic::ToSeed(secure_mnemonic, secure_mnemonic_passphrase, seed_key_tmp);
+
+        CExtKey master_key_tmp;
+        master_key_tmp.SetSeed(MakeByteSpan(seed_key_tmp));
+        assert(master_key == master_key_tmp);
+    }
+
     int64_t creation_time = GetTime();
 
     std::string xpub = EncodeExtPubKey(master_key.Neuter());
@@ -2070,7 +2116,7 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
 
     // Store the master private key, and descriptor
     WalletBatch batch(m_storage.GetDatabase());
-    if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) {
+    if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey(), secure_mnemonic, secure_mnemonic_passphrase)) {
         throw std::runtime_error(std::string(__func__) + ": writing descriptor master private key failed");
     }
     if (!batch.WriteDescriptor(GetID(), m_wallet_descriptor)) {
@@ -2354,21 +2400,34 @@ void DescriptorScriptPubKeyMan::SetCache(const DescriptorCache& cache)
     }
 }
 
-bool DescriptorScriptPubKeyMan::AddKey(const CKeyID& key_id, const CKey& key)
+bool DescriptorScriptPubKeyMan::AddKey(const CKeyID& key_id, const CKey& key, const SecureString& mnemonic, const SecureString& mnemonic_passphrase)
 {
     LOCK(cs_desc_man);
+    if (!mnemonic.empty()) {
+        // TODO: remove duplicated code with AddKey()
+        SecureVector seed_key_tmp;
+        CMnemonic::ToSeed(mnemonic, mnemonic_passphrase, seed_key_tmp);
+
+        CExtKey master_key_tmp;
+        master_key_tmp.SetSeed(MakeByteSpan(seed_key_tmp));
+        assert(key == master_key_tmp.key);
+    }
+
     m_map_keys[key_id] = key;
+    m_mnemonics[key_id] = make_pair(mnemonic, mnemonic_passphrase);
+
     return true;
 }
 
-bool DescriptorScriptPubKeyMan::AddCryptedKey(const CKeyID& key_id, const CPubKey& pubkey, const std::vector<unsigned char>& crypted_key)
+bool DescriptorScriptPubKeyMan::AddCryptedKey(const CKeyID& key_id, const CPubKey& pubkey, const std::vector<unsigned char>& crypted_key, const std::vector<unsigned char>& crypted_mnemonic,const std::vector<unsigned char>& crypted_mnemonic_passphrase)
 {
     LOCK(cs_desc_man);
     if (!m_map_keys.empty()) {
         return false;
     }
 
     m_map_crypted_keys[key_id] = make_pair(pubkey, crypted_key);
+    m_crypted_mnemonics[key_id] = make_pair(crypted_mnemonic, crypted_mnemonic_passphrase);
     return true;
 }
 
@@ -2410,7 +2469,6 @@ bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out, const bool
 
     FlatSigningProvider provider;
     provider.keys = GetKeys();
-
     if (priv) {
         // For the private version, always return the master key to avoid
         // exposing child private keys. The risk implications of exposing child
@@ -2421,6 +2479,65 @@ bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out, const bool
     return m_wallet_descriptor.descriptor->ToNormalizedString(provider, out, &m_wallet_descriptor.cache);
 }
 
+bool DescriptorScriptPubKeyMan::GetMnemonicString(SecureString& mnemonic_out, SecureString& mnemonic_passphrase_out) const
+{
+    LOCK(cs_desc_man);
+
+    mnemonic_out.clear();
+    mnemonic_passphrase_out.clear();
+
+    if (m_mnemonics.empty() && m_crypted_mnemonics.empty()) {
+        WalletLogPrintf("%s: Descriptor wallet has no mnemonic defined\n", __func__);
+        return false;
+    }
+    if (m_storage.IsLocked(false)) return false;
+
+    if (m_mnemonics.size() + m_crypted_mnemonics.size() > 1) {
+        WalletLogPrintf("%s: ERROR: One descriptor has multiple mnemonics. Can't match it\n", __func__);
+        return false;
+    }
+    if (m_storage.HasEncryptionKeys() && !m_storage.IsLocked(true)) {
+        if (!m_crypted_mnemonics.empty() && m_map_crypted_keys.size() != 1) {
+            WalletLogPrintf("%s: ERROR: can't choose encryption key for mnemonic out of %lld\n", __func__, m_map_crypted_keys.size());
+            return false;
+        }
+        const CPubKey& pubkey = m_map_crypted_keys.begin()->second.first;
+        const auto mnemonic = m_crypted_mnemonics.begin()->second;
+        const std::vector<unsigned char>& crypted_mnemonic = mnemonic.first;
+        const std::vector<unsigned char>& crypted_mnemonic_passphrase = mnemonic.second;
+
+        SecureVector mnemonic_v;
+        SecureVector mnemonic_passphrase_v;
+        if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) {
+            return DecryptSecret(encryption_key, crypted_mnemonic, pubkey.GetHash(), mnemonic_v);
+        })) {
+            WalletLogPrintf("%s: ERROR: can't decrypt mnemonic pubkey %s crypted: %s\n", __func__, pubkey.GetHash().ToString(), HexStr(crypted_mnemonic));
+            return false;
+        }
+        if (!crypted_mnemonic_passphrase.empty()) {
+            if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) {
+                return DecryptSecret(encryption_key, crypted_mnemonic_passphrase, pubkey.GetHash(), mnemonic_passphrase_v);
+            })) {
+                WalletLogPrintf("%s: ERROR: can't decrypt mnemonic passphrase\n", __func__);
+                return false;
+            }
+        }
+
+        std::copy(mnemonic_v.begin(), mnemonic_v.end(), std::back_inserter(mnemonic_out));
+        std::copy(mnemonic_passphrase_v.begin(), mnemonic_passphrase_v.end(), std::back_inserter(mnemonic_passphrase_out));
+
+        return true;
+    }
+    if (m_mnemonics.empty()) return false;
+
+    const auto mnemonic_it = m_mnemonics.begin();
+
+    mnemonic_out = mnemonic_it->second.first;
+    mnemonic_passphrase_out = mnemonic_it->second.second;
+
+    return true;
+}
+
 void DescriptorScriptPubKeyMan::UpgradeDescriptorCache()
 {
     LOCK(cs_desc_man);

src/wallet/scriptpubkeyman.h

@@ -510,17 +510,25 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     using CryptedKeyMap = std::map<CKeyID, std::pair<CPubKey, std::vector<unsigned char>>>;
     using KeyMap = std::map<CKeyID, CKey>;
 
+    using Mnemonic = std::pair<SecureString, SecureString>;
+    using MnemonicMap = std::map<CKeyID, Mnemonic>;
+    using CryptedMnemonic = std::pair<std::vector<unsigned char>, std::vector<unsigned char>>;
+    using CryptedMnemonicMap = std::map<CKeyID, CryptedMnemonic>;
+
     ScriptPubKeyMap m_map_script_pub_keys GUARDED_BY(cs_desc_man);
     PubKeyMap m_map_pubkeys GUARDED_BY(cs_desc_man);
     int32_t m_max_cached_index = -1;
 
     KeyMap m_map_keys GUARDED_BY(cs_desc_man);
     CryptedKeyMap m_map_crypted_keys GUARDED_BY(cs_desc_man);
 
+    MnemonicMap m_mnemonics GUARDED_BY(cs_desc_man);
+    CryptedMnemonicMap m_crypted_mnemonics GUARDED_BY(cs_desc_man);
+
     //! keeps track of whether Unlock has run a thorough check before
     bool m_decryption_thoroughly_checked = false;
 
-    bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
+    bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey, const SecureString& mnemonic, const SecureString& mnemonic_passphrase) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
 
     KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
 
@@ -562,7 +570,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     bool IsHDEnabled() const override;
 
     //! Setup descriptors based on the given CExtkey
-    bool SetupDescriptorGeneration(const CExtKey& master_key, bool internal);
+    bool SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, bool internal);
 
     bool HavePrivateKeys() const override;
 
@@ -588,8 +596,8 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
 
     void SetCache(const DescriptorCache& cache);
 
-    bool AddKey(const CKeyID& key_id, const CKey& key);
-    bool AddCryptedKey(const CKeyID& key_id, const CPubKey& pubkey, const std::vector<unsigned char>& crypted_key);
+    bool AddKey(const CKeyID& key_id, const CKey& key, const SecureString& mnemonic, const SecureString& mnemonic_passphrase);
+    bool AddCryptedKey(const CKeyID& key_id, const CPubKey& pubkey, const std::vector<unsigned char>& crypted_key, const std::vector<unsigned char>& crypted_mnemonic,const std::vector<unsigned char>& crypted_mnemonic_passphrase);
 
     bool HasWalletDescriptor(const WalletDescriptor& desc) const;
     void UpdateWalletDescriptor(WalletDescriptor& descriptor);
@@ -601,6 +609,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     const std::vector<CScript> GetScriptPubKeys() const;
 
     bool GetDescriptorString(std::string& out, const bool priv) const;
+    bool GetMnemonicString(SecureString& mnemonic_out, SecureString& mnemonic_passphrase_out) const;
 
     void UpgradeDescriptorCache();
 };