Merge bitcoin/bitcoin#25235: GetExternalSigner(): fail if multiple signers are found

292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0 GetExternalSigner(): fail if multiple signers are found (amadeuszpawlik)

Pull request description:

  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, forcing the user to disconnect all but one external signer, so that there is no ambiguity as to which external signer was used.

ACKs for top commit:
  Sjors:
    tACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0
  achow101:
    ACK 292b1a3e9c98b9ba74b28d149df8554d4ad8e5c0

Tree-SHA512: e2a41d3eecc607d4f94e708614bed0f3545f7abba85f300c5a5f0d3d17d72c815259734accc5ca370953eacd290f27894ba2c18016f5e9584cd50fa1ec2fbb0b

5 files changed, 48 insertions, 6 deletions

diff --git a/test/functional/mocks/invalid_signer.py b/test/functional/mocks/invalid_signer.py
index e30cc9e20b..14f9fed72e 100755
--- a/test/functional/mocks/invalid_signer.py
+++ b/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"
diff --git a/test/functional/mocks/multi_signers.py b/test/functional/mocks/multi_signers.py
new file mode 100755
index 0000000000..88f93e23de
--- /dev/null
+++ b/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)
diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py
index b732b26a53..c5a8f7b1e9 100755
--- a/test/functional/mocks/signer.py
+++ b/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"
diff --git a/test/functional/rpc_signer.py b/test/functional/rpc_signer.py
index f1107197c5..de17b2b929 100755
--- a/test/functional/rpc_signer.py
+++ b/test/functional/rpc_signer.py
@@ -77,10 +77,7 @@ class RPCSignerTest(BitcoinTestFramework):
         )
         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()
diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py
index 8e4e1f5d36..5609ac9bf5 100755
--- a/test/functional/wallet_signer.py
+++ b/test/functional/wallet_signer.py
@@ -32,6 +32,13 @@ class WalletSignerTest(BitcoinTestFramework):
         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 @@ class WalletSignerTest(BitcoinTestFramework):
         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 @@ class WalletSignerTest(BitcoinTestFramework):
         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()