Merge bitcoin/bitcoin#28333: wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively

451fdd26a4 test: wallet: Constructing a DSPKM that can't TopUp() throws. (David Gumberg)
32946e0291 wallet: Setup new autogenerated descriptors on construction (Ava Chow)
e20aaff70f wallet: Construct ExternalSignerSPKM with the new descriptor (Ava Chow)
aa4f7823aa wallet: include keys when constructing DescriptorSPKM during import (Ava Chow)
6538f69135 fuzz: Skip adding descriptor to wallet if it cannot be expanded (Ava Chow)
8be5ee554b test: wallet: Check that loading wallet with both unencrypted and encrypted keys fails. (David Gumberg)
80b0c25992 wallet: Load everything into DescSPKM on construction (Ava Chow)
f713fd1725 refactor: wallet: Don't reuse WALLET_BLANK flag for born-encrypted wallets. (David Gumberg)
cd912c4e10 wallet: Consolidate generation setup callers into one function (Ava Chow)
0301c758ea wallet migration, fuzz: Migrate hd seed once (Ava Chow)

Pull request description:

  Instead of constructing ScriptPubKeyMans with no data, and then loading data as we find it, we should gather everything first and then load it all on construction. If there actually is no data and we want to setup generation, then that should also occur in a constructor rather than afterwards.

  This change is only applied to DescriptorScriptPubKeyMan and ExternalSignerScriptPubKeyMan, and should be done for any ScriptPubKeyMans added in the future. I don't think it's really worth it to do this for LegacyScriptPubKeyMan since it would make loading performance worse (or cause layer violations) and it's (supposed to be) going away soon.

ACKs for top commit:
  polespinasa:
    ACK 451fdd26a4
  davidgumberg:
    re crACK 451fdd26a4
  w0xlt:
    ACK 451fdd26a4

Tree-SHA512: 58a889bf7c77d5da78041907a76a1958207f95a19bec8dc4d86d4e4108d256a729e0949c0973f7d447178f78a7fd4268cda71d358cae4dec5a76dc453b5283af

test/functional/wallet_descriptor.py

@@ -12,6 +12,7 @@ except ImportError:
 import re
 
 from test_framework.blocktools import COINBASE_MATURITY
+from test_framework.messages import ser_string
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_not_equal,
@@ -268,6 +269,25 @@ class WalletDescriptorTest(BitcoinTestFramework):
         conn.close()
         assert_raises_rpc_error(-4, "Unexpected legacy entry in descriptor wallet found.", self.nodes[0].loadwallet, "crashme")
 
+        self.log.info("Test that loading descriptor wallet containing both unencrypted and encrypted keys for same descriptor fails to load")
+        wallet_name = "mixed_crypt"
+        self.nodes[0].createwallet(wallet_name)
+        self.nodes[0].unloadwallet(wallet_name)
+        wallet_db = self.nodes[0].wallets_path / wallet_name / self.wallet_data_filename
+        conn = sqlite3.connect(wallet_db)
+        with conn:
+            key_prefix = ser_string(b"walletdescriptorkey")
+            ckey_prefix = ser_string(b"walletdescriptorckey")
+            rows = conn.execute('SELECT key, value FROM main').fetchall()
+            key_rows = [(k, v) for k, v in rows if k.startswith(key_prefix)]
+            # Test the test, want to be sure there is at least one unencrypted key.
+            assert len(key_rows) >= 1
+            k, v = key_rows[0]
+            conn.execute('INSERT INTO main VALUES(?, ?)', (k.replace(key_prefix, ckey_prefix), v))
+        conn.close()
+        with self.nodes[0].assert_debug_log(["Wallet contains both unencrypted and encrypted keys"]):
+            assert_raises_rpc_error(-4, "Wallet corrupted", self.nodes[0].loadwallet, wallet_name)
+
         self.test_parent_descriptors()
 
 if __name__ == '__main__':