Merge bitcoin/bitcoin#26462: wallet: fix crash on loading descriptor wallet containing legacy key type entries

3198e4239e848bbb119e3638677aa9bcf8353ca6 test: check that loading descriptor wallet with legacy entries throws error (Sebastian Falbesoner)
349ed2a0eed3aaaf199ead93057c97730869c3a3 wallet: throw error if legacy entries are present on loading descriptor wallets (Sebastian Falbesoner)

Pull request description:

  Loading a descriptor wallet currently leads to a segfault if a legacy key type entry is present that can be deserialized successfully and needs SPKman-interaction. To reproduce with a "cscript" entry (see second commit for details):

  ```
  $ ./src/bitcoin-cli createwallet crashme
  $ ./src/bitcoin-cli unloadwallet crashme
  $ sqlite3 ~/.bitcoin/wallets/crashme/wallet.dat
  SQLite version 3.38.2 2022-03-26 13:51:10
  Enter ".help" for usage hints.
  sqlite> INSERT INTO main VALUES(x'07637363726970740000000000000000000000000000000000000000', x'00');
  $ ./src/bitcoin-cli loadwallet crashme

  --- bitcoind output: ---
  2022-11-06T13:51:01Z Using SQLite Version 3.38.2
  2022-11-06T13:51:01Z Using wallet /home/honey/.bitcoin/wallets/crashme
  2022-11-06T13:51:01Z init message: Loading wallet…
  2022-11-06T13:51:01Z [crashme] Wallet file version = 10500, last client version = 249900

  Segmentation fault (core dumped)
  ```

  Background: In the wallet key-value-loading routine, most legacy type entries require a `LegacyScriptPubKeyMan` instance after successful deserialization. On a descriptor wallet, creating that (via method `GetOrCreateLegacyScriptPubKeyMan`) fails and then leads to a null-pointer dereference crash. E.g. for CSCRIPT: https://github.com/bitcoin/bitcoin/blob/50422b770a40f5fa964201d1e99fd6b5dc1653ca/src/wallet/walletdb.cpp#L589-L594

  ~~This PR fixes this by simply ignoring legacy entries if the wallet flags indicate that we have a descriptor wallet. The second commits adds a regression test to the descriptor wallet's functional test (fortunately Python includes sqlite3 support in the standard library).~~

  ~~Probably it would be even better to throw a warning to the user if unexpected legacy entries are found in descriptor wallets, but I think as a first mitigation everything is obvisouly better than crashing. As far as I'm aware, descriptor wallets created/migrated by Bitcoin Core should never end up in a state containing legacy type entries though.~~

  This PR fixes this by throwing an error if legacy entries are found in descriptor wallets on loading.

ACKs for top commit:
  achow101:
    ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6
  aureleoules:
    ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6

Tree-SHA512: ee43da3f61248e0fde55d9a705869202cb83df678ebf4816f0e77263f0beac0d7bae9490465d1753159efb093ee37182931d76b2e2b6e8c6f8761285700ace1c

1 files changed, 12 insertions, 0 deletions

diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py
index 4b305106f1..8692fa1968 100755
--- a/test/functional/wallet_descriptor.py
+++ b/test/functional/wallet_descriptor.py
@@ -3,6 +3,8 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test descriptor wallet function."""
+import os
+import sqlite3
 
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
@@ -224,5 +226,15 @@ class WalletDescriptorTest(BitcoinTestFramework):
                     imp_addr = imp_rpc.getnewaddress(address_type=addr_type)
                 assert_equal(exp_addr, imp_addr)
 
+        self.log.info("Test that loading descriptor wallet containing legacy key types throws error")
+        self.nodes[0].createwallet(wallet_name="crashme", descriptors=True)
+        self.nodes[0].unloadwallet("crashme")
+        wallet_db = os.path.join(self.nodes[0].datadir, self.chain, "wallets", "crashme", self.wallet_data_filename)
+        with sqlite3.connect(wallet_db) as conn:
+            # add "cscript" entry: key type is uint160 (20 bytes), value type is CScript (zero-length here)
+            conn.execute('INSERT INTO main VALUES(?, ?)', (b'\x07cscript' + b'\x00'*20, b'\x00'))
+        assert_raises_rpc_error(-4, "Unexpected legacy entry in descriptor wallet found.", self.nodes[0].loadwallet, "crashme")
+
+
 if __name__ == '__main__':
     WalletDescriptorTest().main ()