Skip to content

Commit c74e570

Browse files
ryanofskyryanofsky
committed
multiprocess: Run external signer in wallet not node process
Add interfaces::ExternalSigner to allow signer objects to be referenced and controlled in different processes. Always run external signer code in bitcoin-wallet process, not bitcoin-gui or bitcoin-node processes.
1 parent 6ef84e0 commit c74e570

File tree

7 files changed

+58
-28
lines changed

7 files changed

+58
-28
lines changed

‎src/interfaces/node.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#define BITCOIN_INTERFACES_NODE_H
77

88
#include <amount.h> // For CAmount
9-
#include <external_signer.h>
109
#include <net.h> // For NodeId
1110
#include <net_types.h> // For banmap_t
1211
#include <netaddress.h> // For Network
@@ -111,9 +110,6 @@ class Node
111110
//! Disconnect node by id.
112111
virtual bool disconnectById(NodeId id) = 0;
113112

114-
//! List external signers
115-
virtual std::vector<ExternalSigner> externalSigners() = 0;
116-
117113
//! Get total bytes recv.
118114
virtual int64_t getTotalBytesRecv() = 0;
119115

‎src/interfaces/wallet.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,16 @@ class Wallet
306306
virtual CWallet* wallet() { return nullptr; }
307307
};
308308

309+
//! External signer interface used by the GUI.
310+
class ExternalSigner
311+
{
312+
public:
313+
virtual ~ExternalSigner() {};
314+
315+
//! Get signer display name
316+
virtual std::string getName() = 0;
317+
};
318+
309319
//! Wallet chain client that in addition to having chain client methods for
310320
//! starting up, shutting down, and registering RPCs, also has additional
311321
//! methods (called by the GUI) to load and create wallets.
@@ -327,6 +337,9 @@ class WalletClient : public ChainClient
327337
//! Return interfaces for accessing wallets (if any).
328338
virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
329339

340+
//! Return list of external signers (attached devices which can sign transactions).
341+
virtual std::vector<std::unique_ptr<ExternalSigner>> listExternalSigners() = 0;
342+
330343
//! Register handler for load wallet messages. This callback is triggered by
331344
//! createWallet and loadWallet above, and also triggered when wallets are
332345
//! loaded at startup or by RPC.

‎src/node/interfaces.cpp‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -172,24 +172,6 @@ class NodeImpl : public Node
172172
}
173173
return false;
174174
}
175-
std::vector<ExternalSigner> externalSigners() override
176-
{
177-
#ifdef ENABLE_EXTERNAL_SIGNER
178-
std::vector<ExternalSigner> signers = {};
179-
const std::string command = gArgs.GetArg("-signer", "");
180-
if (command == "") return signers;
181-
ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
182-
return signers;
183-
#else
184-
// This result is indistinguishable from a successful call that returns
185-
// no signers. For the current GUI this doesn't matter, because the wallet
186-
// creation dialog disables the external signer checkbox in both
187-
// cases. The return type could be changed to std::optional<std::vector>
188-
// (or something that also includes error messages) if this distinction
189-
// becomes important.
190-
return {};
191-
#endif // ENABLE_EXTERNAL_SIGNER
192-
}
193175
int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; }
194176
int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; }
195177
size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; }

‎src/qt/createwalletdialog.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#include <config/bitcoin-config.h>
77
#endif
88

9-
#include <external_signer.h>
9+
#include <interfaces/wallet.h>
1010
#include <qt/createwalletdialog.h>
1111
#include <qt/forms/ui_createwalletdialog.h>
1212

@@ -113,7 +113,7 @@ CreateWalletDialog::~CreateWalletDialog()
113113
delete ui;
114114
}
115115

116-
void CreateWalletDialog::setSigners(const std::vector<ExternalSigner>& signers)
116+
void CreateWalletDialog::setSigners(const std::vector<std::unique_ptr<interfaces::ExternalSigner>>& signers)
117117
{
118118
m_has_signers = !signers.empty();
119119
if (m_has_signers) {
@@ -126,7 +126,7 @@ void CreateWalletDialog::setSigners(const std::vector<ExternalSigner>& signers)
126126
ui->blank_wallet_checkbox->setChecked(false);
127127
ui->disable_privkeys_checkbox->setEnabled(false);
128128
ui->disable_privkeys_checkbox->setChecked(true);
129-
const std::string label = signers[0].m_name;
129+
const std::string label = signers[0]->getName();
130130
ui->wallet_name_line_edit->setText(QString::fromStdString(label));
131131
ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
132132
} else {

‎src/qt/createwalletdialog.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77

88
#include <QDialog>
99

10+
#include <memory>
11+
12+
namespace interfaces {
1013
class ExternalSigner;
14+
} // namespace interfaces
15+
1116
class WalletModel;
1217

1318
namespace Ui {
@@ -24,7 +29,7 @@ class CreateWalletDialog : public QDialog
2429
explicit CreateWalletDialog(QWidget* parent);
2530
virtual ~CreateWalletDialog();
2631

27-
void setSigners(const std::vector<ExternalSigner>& signers);
32+
void setSigners(const std::vector<std::unique_ptr<interfaces::ExternalSigner>>& signers);
2833

2934
QString walletName() const;
3035
bool isEncryptWalletChecked() const;

‎src/qt/walletcontroller.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ void CreateWalletActivity::create()
296296
{
297297
m_create_wallet_dialog = new CreateWalletDialog(m_parent_widget);
298298

299-
std::vector<ExternalSigner> signers;
299+
std::vector<std::unique_ptr<interfaces::ExternalSigner>> signers;
300300
try {
301-
signers = node().externalSigners();
301+
signers = node().walletClient().listExternalSigners();
302302
} catch (const std::runtime_error& e) {
303303
QMessageBox::critical(nullptr, tr("Can't list signers"), e.what());
304304
}

‎src/wallet/interfaces.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <interfaces/wallet.h>
66

77
#include <amount.h>
8+
#include <chainparams.h>
9+
#include <external_signer.h>
810
#include <interfaces/chain.h>
911
#include <interfaces/handler.h>
1012
#include <policy/fees.h>
@@ -500,6 +502,16 @@ class WalletImpl : public Wallet
500502
std::shared_ptr<CWallet> m_wallet;
501503
};
502504

505+
class ExternalSignerImpl : public interfaces::ExternalSigner
506+
{
507+
public:
508+
#ifdef ENABLE_EXTERNAL_SIGNER
509+
ExternalSignerImpl(::ExternalSigner signer) : m_signer(std::move(signer)) {}
510+
std::string getName() override { return m_signer.m_name; }
511+
::ExternalSigner m_signer;
512+
#endif
513+
};
514+
503515
class WalletClientImpl : public WalletClient
504516
{
505517
public:
@@ -559,6 +571,28 @@ class WalletClientImpl : public WalletClient
559571
}
560572
return paths;
561573
}
574+
std::vector<std::unique_ptr<interfaces::ExternalSigner>> listExternalSigners() override
575+
{
576+
#ifdef ENABLE_EXTERNAL_SIGNER
577+
const std::string command = gArgs.GetArg("-signer", "");
578+
if (command == "") return {};
579+
std::vector<::ExternalSigner> signers;
580+
ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
581+
std::vector<std::unique_ptr<interfaces::ExternalSigner>> result;
582+
for (auto& signer : signers) {
583+
result.emplace_back(std::make_unique<ExternalSignerImpl>(std::move(signer)));
584+
}
585+
return result;
586+
#else
587+
// This result is indistinguishable from a successful call that returns
588+
// no signers. For the current GUI this doesn't matter, because the wallet
589+
// creation dialog disables the external signer checkbox in both
590+
// cases. The return type could be changed to std::optional<std::vector>
591+
// (or something that also includes error messages) if this distinction
592+
// becomes important.
593+
return {};
594+
#endif // ENABLE_EXTERNAL_SIGNER
595+
}
562596
std::vector<std::unique_ptr<Wallet>> getWallets() override
563597
{
564598
std::vector<std::unique_ptr<Wallet>> wallets;

0 commit comments

Comments
 (0)