Check descriptors returned by external signers

Check that descriptors returned by external signers have been parsed properly when creating a new wallet.

src/wallet/wallet.cpp

@@ -3203,8 +3203,11 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
             for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) {
                 std::string desc_str = desc_val.getValStr();
                 FlatSigningProvider keys;
-                std::string dummy_error;
-                std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, dummy_error, false);
+                std::string desc_error;
+                std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, desc_error, false);
+                if (desc == nullptr) {
+                    throw std::runtime_error(std::string(__func__) + ": Invalid descriptor \"" + desc_str + "\" (" + desc_error + ")");
+                }
                 if (!desc->GetOutputType()) {
                     continue;
                 }

test/functional/mocks/invalid_signer.py

@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+# Copyright (c) 2018-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+import os
+import sys
+import argparse
+import json
+
+def perform_pre_checks():
+    mock_result_path = os.path.join(os.getcwd(), "mock_result")
+    if(os.path.isfile(mock_result_path)):
+        with open(mock_result_path, "r", encoding="utf8") as f:
+            mock_result = f.read()
+        if mock_result[0]:
+            sys.stdout.write(mock_result[2:])
+            sys.exit(int(mock_result[0]))
+
+def enumerate(args):
+    sys.stdout.write(json.dumps([{"fingerprint": "b3c19bfc", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}]))
+
+def getdescriptors(args):
+    xpub_pkh = "xpub6CRhJvXV8x2AKWvqi1ZSMFU6cbxzQiYrv3dxSUXCawjMJ1JzpqVsveH4way1yCmJm29KzH1zrVZmVwes4Qo6oXVE1HFn4fdiKrYJngqFFc6"
+    xpub_sh = "xpub6CoNoq3Tg4tGSpom2BSwL42gy864KHo3TXkHxLxBbhvCkgmdVXADQmiHbLZhX3Me1cYhRx7s25Lpm4LnT5zu395ANHsXB2QvT9tqJDAibTN"
+    xpub_wpkh = "xpub6DUcLgY1DfgDy2RV6q4djwwsLitaoZDumbribqrR8mP78fEtgZa1XEsqT5MWQ7gwLwKsTQPT28XLoVE5A97rDNTwMXjmzPaNijoCApCbWvp"
+
+    sys.stdout.write(json.dumps({
+        "receive": [
+            "pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/0/*)#h26nxtl9",
+            "sh(wpkh([b3c19bfc/49'/1'/" + args.account + "']" + xpub_sh + "/0/*))#32ry02yp",
+            "wpkh([b3c19bfc/84'/1'/" + args.account + "']" + xpub_wpkh + "/0/*)#jftn8ppv"
+        ],
+        "internal": [
+            "pkh([b3c19bfc/44'/1'/" + args.account + "']" + xpub_pkh + "/1/*)#x7ljm70a",
+            "sh(wpkh([b3c19bfc/49'/1'/" + args.account + "']" + xpub_sh + "/1/*))#ytdjh437",
+            "wpkh([b3c19bfc/84'/1'/" + args.account + "']" + xpub_wpkh + "/1/*)#rawj6535"
+        ]
+    }))
+
+parser = argparse.ArgumentParser(prog='./invalid_signer.py', description='External invalid signer mock')
+parser.add_argument('--fingerprint')
+parser.add_argument('--chain', default='main')
+parser.add_argument('--stdin', action='store_true')
+
+subparsers = parser.add_subparsers(description='Commands', dest='command')
+subparsers.required = True
+
+parser_enumerate = subparsers.add_parser('enumerate', help='list available signers')
+parser_enumerate.set_defaults(func=enumerate)
+
+parser_getdescriptors = subparsers.add_parser('getdescriptors')
+parser_getdescriptors.set_defaults(func=getdescriptors)
+parser_getdescriptors.add_argument('--account', metavar='account')
+
+if not sys.stdin.isatty():
+    buffer = sys.stdin.read()
+    if buffer and buffer.rstrip() != "":
+        sys.argv.extend(buffer.rstrip().split(" "))
+
+args = parser.parse_args()
+
+perform_pre_checks()
+
+args.func(args)

test/functional/wallet_signer.py

@@ -25,6 +25,13 @@ class WalletSignerTest(BitcoinTestFramework):
         else:
             return path
 
+    def mock_invalid_signer_path(self):
+        path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'invalid_signer.py')
+        if platform.system() == "Windows":
+            return "py " + path
+        else:
+            return path
+
     def set_test_params(self):
         self.num_nodes = 2
         # The experimental syscall sandbox feature (-sandbox) is not compatible with -signer (which
@@ -48,6 +55,11 @@ class WalletSignerTest(BitcoinTestFramework):
         os.remove(os.path.join(node.cwd, "mock_result"))
 
     def run_test(self):
+        self.test_valid_signer()
+        self.restart_node(1, [f"-signer={self.mock_invalid_signer_path()}", "-keypool=10"])
+        self.test_invalid_signer()
+
+    def test_valid_signer(self):
         self.log.debug(f"-signer={self.mock_signer_path()}")
 
         # Create new wallets for an external signer.
@@ -187,5 +199,10 @@ class WalletSignerTest(BitcoinTestFramework):
         # )
         # self.clear_mock_result(self.nodes[4])
 
+    def test_invalid_signer(self):
+        self.log.debug(f"-signer={self.mock_invalid_signer_path()}")
+        self.log.info('Test invalid external signer')
+        assert_raises_rpc_error(-1, "Invalid descriptor", self.nodes[1].createwallet, wallet_name='hww_invalid', disable_private_keys=True, descriptors=True, external_signer=True)
+
 if __name__ == '__main__':
     WalletSignerTest().main()