GetExternalSigner(): fail if multiple signers are found

If there are multiple external signers, `GetExternalSigner()` will
just pick the first one in the list. If the user has two or more
hardware wallets connected at the same time, he might not notice this.

This PR adds a check and fails with suitable message.

Author: amadeuszpawlik

src/qt/walletcontroller.cpp

@@ -293,6 +293,10 @@ void CreateWalletActivity::create()
     } catch (const std::runtime_error& e) {
         QMessageBox::critical(nullptr, tr("Can't list signers"), e.what());
     }
+    if (signers.size() > 1) {
+        QMessageBox::critical(nullptr, tr("Too many external signers found"), QString::fromStdString("More than one external signer found. Please connect only one at a time."));
+        signers.clear();
+    }
     m_create_wallet_dialog->setSigners(signers);
 
     m_create_wallet_dialog->setWindowModality(Qt::ApplicationModal);

src/wallet/external_signer_scriptpubkeyman.cpp

@@ -45,7 +45,8 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() {
     std::vector<ExternalSigner> signers;
     ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
     if (signers.empty()) throw std::runtime_error(std::string(__func__) + ": No external signers found");
-    // TODO: add fingerprint argument in case of multiple signers
+    // TODO: add fingerprint argument instead of failing in case of multiple signers.
+    if (signers.size() > 1) throw std::runtime_error(std::string(__func__) + ": More than one external signer found. Please connect only one at a time.");
     return signers[0];
 }
 

test/functional/mocks/invalid_signer.py

@@ -18,7 +18,7 @@ def perform_pre_checks():
             sys.exit(int(mock_result[0]))
 
 def enumerate(args):
-    sys.stdout.write(json.dumps([{"fingerprint": "b3c19bfc", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}]))
+    sys.stdout.write(json.dumps([{"fingerprint": "b3c19bfc", "type": "trezor", "model": "trezor_t"}]))
 
 def getdescriptors(args):
     xpub_pkh = "xpub6CRhJvXV8x2AKWvqi1ZSMFU6cbxzQiYrv3dxSUXCawjMJ1JzpqVsveH4way1yCmJm29KzH1zrVZmVwes4Qo6oXVE1HFn4fdiKrYJngqFFc6"

test/functional/mocks/multi_signers.py

@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022 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 argparse
+import json
+import sys
+
+def enumerate(args):
+    sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"},
+        {"fingerprint": "00000002", "type": "trezor", "model": "trezor_one"}]))
+
+parser = argparse.ArgumentParser(prog='./multi_signers.py', description='External multi-signer mock')
+
+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)
+
+
+if not sys.stdin.isatty():
+    buffer = sys.stdin.read()
+    if buffer and buffer.rstrip() != "":
+        sys.argv.extend(buffer.rstrip().split(" "))
+
+args = parser.parse_args()
+
+args.func(args)

test/functional/mocks/signer.py

@@ -18,7 +18,7 @@ def perform_pre_checks():
             sys.exit(int(mock_result[0]))
 
 def enumerate(args):
-    sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}, {"fingerprint": "00000002"}]))
+    sys.stdout.write(json.dumps([{"fingerprint": "00000001", "type": "trezor", "model": "trezor_t"}]))
 
 def getdescriptors(args):
     xpub = "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B"

test/functional/rpc_signer.py

@@ -77,10 +77,7 @@ def run_test(self):
         )
         self.clear_mock_result(self.nodes[1])
 
-        result = self.nodes[1].enumeratesigners()
-        assert_equal(len(result['signers']), 2)
-        assert_equal(result['signers'][0]["fingerprint"], "00000001")
-        assert_equal(result['signers'][0]["name"], "trezor_t")
+        assert_equal({'fingerprint': '00000001', 'name': 'trezor_t'} in self.nodes[1].enumeratesigners()['signers'], True)
 
 if __name__ == '__main__':
     RPCSignerTest().main()

test/functional/wallet_signer.py

@@ -32,6 +32,13 @@ def mock_invalid_signer_path(self):
         else:
             return path
 
+    def mock_multi_signers_path(self):
+        path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'multi_signers.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
@@ -58,6 +65,8 @@ 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()
+        self.restart_node(1, [f"-signer={self.mock_multi_signers_path()}", "-keypool=10"])
+        self.test_multiple_signers()
 
     def test_valid_signer(self):
         self.log.debug(f"-signer={self.mock_signer_path()}")
@@ -212,5 +221,11 @@ def test_invalid_signer(self):
         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)
 
+    def test_multiple_signers(self):
+        self.log.debug(f"-signer={self.mock_multi_signers_path()}")
+        self.log.info('Test multiple external signers')
+
+        assert_raises_rpc_error(-1, "GetExternalSigner: More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', disable_private_keys=True, descriptors=True, external_signer=True)
+
 if __name__ == '__main__':
     WalletSignerTest().main()