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

292b1a3e9c 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 292b1a3e9c
  achow101:
    ACK 292b1a3e9c

Tree-SHA512: e2a41d3eecc607d4f94e708614bed0f3545f7abba85f300c5a5f0d3d17d72c815259734accc5ca370953eacd290f27894ba2c18016f5e9584cd50fa1ec2fbb0b
pull/25849/head
fanquake 1 month ago
commit dc9d662683
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
  1. 4
      src/qt/walletcontroller.cpp
  2. 3
      src/wallet/external_signer_scriptpubkeyman.cpp
  3. 2
      test/functional/mocks/invalid_signer.py
  4. 30
      test/functional/mocks/multi_signers.py
  5. 2
      test/functional/mocks/signer.py
  6. 5
      test/functional/rpc_signer.py
  7. 15
      test/functional/wallet_signer.py

@ -297,6 +297,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);

@ -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];
}

@ -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"

@ -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)

@ -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"

@ -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()

@ -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()

Loading…
Cancel
Save