Skip to content

Commit 38911c7

Browse files
meshcolliderknst
meshcollider
authored and
knst
committed
Merge bitcoin-core/gui#4: UI external signer support (e.g. hardware wallet)
1c4b456 gui: send using external signer (Sjors Provoost) 24815c6 gui: wallet creation detects external signer (Sjors Provoost) 3f845ea node: add externalSigners to interface (Sjors Provoost) 62ac119 gui: display address on external signer (Sjors Provoost) 450cb40 wallet: add displayAddress to interface (Sjors Provoost) eef8d64 gui: create wallet with external signer (Sjors Provoost) 6cdbc83 gui: add external signer path to options dialog (Sjors Provoost) Pull request description: Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d). This PR adds GUI support for external signers, based on the since merged bitcoin#16546 (RPC). The UX isn't amazing - especially the blocking calls - but it works. First we adds a GUI setting for the signer script (e.g. path to HWI): <img width="625" alt="Schermafbeelding 2019-08-05 om 19 32 59" src="https://user-images.githubusercontent.com/10217/62483415-e1ff1680-b7b7-11e9-97ca-8d2ce54ca1cb.png"> Then we add an external signer checkbox to the wallet creation dialog: <img width="374" alt="Schermafbeelding 2019-11-07 om 19 17 23" src="https://user-images.githubusercontent.com/10217/68416387-b57ee000-0194-11ea-9730-127d60273008.png"> It's checked by default if HWI detects a device. It also grabs the name. It then creates a fresh wallet and imports the keys. You can verify an address on the device (blocking...): <img width="673" alt="Schermafbeelding 2019-08-05 om 19 29 22" src="https://user-images.githubusercontent.com/10217/62483560-43bf8080-b7b8-11e9-9902-8a036116dc4b.png"> Sending, including coin selection, Just Works(tm) as long the device is present. ~External signer support is enabled by default when the GUI is configured and Boost::Process is present.~ External signer support remains disabled by default, see bitcoin#21935. ACKs for top commit: achow101: Code Review ACK 1c4b456 hebasto: ACK 1c4b456, tested on Linux Mint 20.1 (Qt 5.12.8) with HWW `2.0.2-rc.1`. promag: Tested ACK 1c4b456 but rebased with e033ca1, with HWI 2.0.2, with Nano S and Nano X. meshcollider: re-code-review ACK 1c4b456 Tree-SHA512: 3503113c5c69d40adb6ce364d8e7cae23ce82d032a00474ba9aeb6202eb70f496ef4a6bf2e623e5171e524ad31ade7941a4e0e89539c64518aaec74f4562d86b
1 parent 2ebf7ca commit 38911c7

19 files changed

+280
-13
lines changed

‎src/interfaces/node.h‎

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

88
#include <consensus/amount.h> // For CAmount
9+
#include <external_signer.h>
910
#include <net.h> // For NodeId
1011
#include <net_types.h> // For banmap_t
1112
#include <netaddress.h> // For Network
@@ -240,6 +241,11 @@ class Node
240241
//! Disconnect node by id.
241242
virtual bool disconnectById(NodeId id) = 0;
242243

244+
#ifdef ENABLE_EXTERNAL_SIGNER
245+
//! List external signers
246+
virtual std::vector<ExternalSigner> externalSigners() = 0;
247+
#endif
248+
243249
//! Get total bytes recv.
244250
virtual int64_t getTotalBytesRecv() = 0;
245251

‎src/interfaces/wallet.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ class Wallet
151151
//! Save or remove receive request.
152152
virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0;
153153

154+
//! Display address on external signer
155+
virtual bool displayAddress(const CTxDestination& dest) = 0;
156+
154157
//! Lock coin.
155158
virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0;
156159

@@ -288,6 +291,9 @@ class Wallet
288291
// Return whether private keys enabled.
289292
virtual bool privateKeysDisabled() = 0;
290293

294+
// Return whether wallet uses an external signer.
295+
virtual bool hasExternalSigner() = 0;
296+
291297
//! Get max tx fee.
292298
virtual CAmount getDefaultMaxTxFee() = 0;
293299

‎src/node/interfaces.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,16 @@ class NodeImpl : public Node
555555
}
556556
return false;
557557
}
558+
#ifdef ENABLE_EXTERNAL_SIGNER
559+
std::vector<ExternalSigner> externalSigners() override
560+
{
561+
std::vector<ExternalSigner> signers = {};
562+
const std::string command = gArgs.GetArg("-signer", "");
563+
if (command == "") return signers;
564+
ExternalSigner::Enumerate(command, signers, Params().NetworkIDString());
565+
return signers;
566+
}
567+
#endif
558568
int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; }
559569
int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; }
560570
size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; }

‎src/qt/createwalletdialog.cpp‎

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

9+
#include <external_signer.h>
910
#include <qt/createwalletdialog.h>
1011
#include <qt/forms/ui_createwalletdialog.h>
1112

@@ -39,14 +40,39 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) :
3940
});
4041

4142
connect(ui->encrypt_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) {
42-
// Disable the disable_privkeys_checkbox when isEncryptWalletChecked is
43+
// Disable the disable_privkeys_checkbox and external_signer_checkbox when isEncryptWalletChecked is
4344
// set to true, enable it when isEncryptWalletChecked is false.
4445
ui->disable_privkeys_checkbox->setEnabled(!checked);
46+
ui->external_signer_checkbox->setEnabled(!checked);
4547

4648
// When the disable_privkeys_checkbox is disabled, uncheck it.
4749
if (!ui->disable_privkeys_checkbox->isEnabled()) {
4850
ui->disable_privkeys_checkbox->setChecked(false);
4951
}
52+
53+
// When the external_signer_checkbox box is disabled, uncheck it.
54+
if (!ui->external_signer_checkbox->isEnabled()) {
55+
ui->external_signer_checkbox->setChecked(false);
56+
}
57+
58+
});
59+
60+
connect(ui->external_signer_checkbox, &QCheckBox::toggled, [this](bool checked) {
61+
ui->encrypt_wallet_checkbox->setEnabled(!checked);
62+
ui->blank_wallet_checkbox->setEnabled(!checked);
63+
ui->disable_privkeys_checkbox->setEnabled(!checked);
64+
ui->descriptor_checkbox->setEnabled(!checked);
65+
66+
// The external signer checkbox is only enabled when a device is detected.
67+
// In that case it is checked by default. Toggling it restores the other
68+
// options to their default.
69+
ui->descriptor_checkbox->setChecked(checked);
70+
ui->encrypt_wallet_checkbox->setChecked(false);
71+
ui->disable_privkeys_checkbox->setChecked(checked);
72+
// The blank check box is ambiguous. This flag is always true for a
73+
// watch-only wallet, even though we immedidately fetch keys from the
74+
// external signer.
75+
ui->blank_wallet_checkbox->setChecked(checked);
5076
});
5177
connect(ui->disable_privkeys_checkbox, &QCheckBox::toggled, [this](bool checked) {
5278
// Disable the encrypt_wallet_checkbox when isDisablePrivateKeysChecked is
@@ -74,18 +100,51 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) :
74100
ui->descriptor_checkbox->setToolTip(tr("Compiled without sqlite support (required for descriptor wallets)"));
75101
ui->descriptor_checkbox->setEnabled(false);
76102
ui->descriptor_checkbox->setChecked(false);
103+
ui->external_signer_checkbox->setEnabled(false);
104+
ui->external_signer_checkbox->setChecked(false);
77105
#endif
106+
78107
#ifndef USE_BDB
79108
ui->descriptor_checkbox->setEnabled(false);
80109
ui->descriptor_checkbox->setChecked(true);
81110
#endif
111+
112+
#ifndef ENABLE_EXTERNAL_SIGNER
113+
//: "External signing" means using devices such as hardware wallets.
114+
ui->external_signer_checkbox->setToolTip(tr("Compiled without external signing support (required for external signing)"));
115+
ui->external_signer_checkbox->setEnabled(false);
116+
ui->external_signer_checkbox->setChecked(false);
117+
#endif
118+
82119
}
83120

84121
CreateWalletDialog::~CreateWalletDialog()
85122
{
86123
delete ui;
87124
}
88125

126+
#ifdef ENABLE_EXTERNAL_SIGNER
127+
void CreateWalletDialog::setSigners(std::vector<ExternalSigner>& signers)
128+
{
129+
if (!signers.empty()) {
130+
ui->external_signer_checkbox->setEnabled(true);
131+
ui->external_signer_checkbox->setChecked(true);
132+
ui->encrypt_wallet_checkbox->setEnabled(false);
133+
ui->encrypt_wallet_checkbox->setChecked(false);
134+
// The order matters, because connect() is called when toggling a checkbox:
135+
ui->blank_wallet_checkbox->setEnabled(false);
136+
ui->blank_wallet_checkbox->setChecked(false);
137+
ui->disable_privkeys_checkbox->setEnabled(false);
138+
ui->disable_privkeys_checkbox->setChecked(true);
139+
const std::string label = signers[0].m_name;
140+
ui->wallet_name_line_edit->setText(QString::fromStdString(label));
141+
ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
142+
} else {
143+
ui->external_signer_checkbox->setEnabled(false);
144+
}
145+
}
146+
#endif
147+
89148
QString CreateWalletDialog::walletName() const
90149
{
91150
return ui->wallet_name_line_edit->text();
@@ -110,3 +169,8 @@ bool CreateWalletDialog::isDescriptorWalletChecked() const
110169
{
111170
return ui->descriptor_checkbox->isChecked();
112171
}
172+
173+
bool CreateWalletDialog::isExternalSignerChecked() const
174+
{
175+
return ui->external_signer_checkbox->isChecked();
176+
}

‎src/qt/createwalletdialog.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99

1010
class WalletModel;
1111

12+
#ifdef ENABLE_EXTERNAL_SIGNER
13+
class ExternalSigner;
14+
#endif
15+
1216
namespace Ui {
1317
class CreateWalletDialog;
1418
}
@@ -23,11 +27,16 @@ class CreateWalletDialog : public QDialog
2327
explicit CreateWalletDialog(QWidget* parent);
2428
virtual ~CreateWalletDialog();
2529

30+
#ifdef ENABLE_EXTERNAL_SIGNER
31+
void setSigners(std::vector<ExternalSigner>& signers);
32+
#endif
33+
2634
QString walletName() const;
2735
bool isEncryptWalletChecked() const;
2836
bool isDisablePrivateKeysChecked() const;
2937
bool isMakeBlankWalletChecked() const;
3038
bool isDescriptorWalletChecked() const;
39+
bool isExternalSignerChecked() const;
3140

3241
private:
3342
Ui::CreateWalletDialog *ui;

‎src/qt/forms/createwalletdialog.ui‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,16 @@
155155
</property>
156156
</widget>
157157
</item>
158+
<item>
159+
<widget class="QCheckBox" name="external_signer_checkbox">
160+
<property name="toolTip">
161+
<string>Use an external signing device such as a hardware wallet. Configure the external signer script in wallet preferences first.</string>
162+
</property>
163+
<property name="text">
164+
<string>External signer</string>
165+
</property>
166+
</widget>
167+
</item>
158168
</layout>
159169
</widget>
160170
</item>
@@ -188,6 +198,7 @@
188198
<tabstop>encrypt_wallet_checkbox</tabstop>
189199
<tabstop>disable_privkeys_checkbox</tabstop>
190200
<tabstop>blank_wallet_checkbox</tabstop>
201+
<tabstop>external_signer_checkbox</tabstop>
191202
</tabstops>
192203
<resources/>
193204
<connections>

‎src/qt/forms/optionsdialog.ui‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,36 @@
409409
</item>
410410
</layout>
411411
</item>
412+
<item>
413+
<widget class="QGroupBox" name="groupBoxHww">
414+
<property name="title">
415+
<string>External Signer (e.g. hardware wallet)</string>
416+
</property>
417+
<layout class="QVBoxLayout" name="verticalLayoutHww">
418+
<item>
419+
<layout class="QHBoxLayout" name="horizontalLayoutHww">
420+
<item>
421+
<widget class="QLabel" name="externalSignerPathLabel">
422+
<property name="text">
423+
<string>&amp;External signer script path</string>
424+
</property>
425+
<property name="buddy">
426+
<cstring>externalSignerPath</cstring>
427+
</property>
428+
</widget>
429+
</item>
430+
<item>
431+
<widget class="QLineEdit" name="externalSignerPath">
432+
<property name="toolTip">
433+
<string>Full path to a Dash Core compatible script (e.g. C:\Downloads\hwi.exe or /Users/you/Downloads/hwi.py). Beware: malware can steal your coins!</string>
434+
</property>
435+
</widget>
436+
</item>
437+
</layout>
438+
</item>
439+
</layout>
440+
</widget>
441+
</item>
412442
<item>
413443
<spacer name="verticalSpacer_Wallet">
414444
<property name="orientation">

‎src/qt/forms/receiverequestdialog.ui‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,19 @@
254254
</property>
255255
</widget>
256256
</item>
257+
<item>
258+
<widget class="QPushButton" name="btnVerify">
259+
<property name="text">
260+
<string>&amp;Verify</string>
261+
</property>
262+
<property name="toolTip">
263+
<string>Verify this address on e.g. a hardware wallet screen</string>
264+
</property>
265+
<property name="autoDefault">
266+
<bool>false</bool>
267+
</property>
268+
</widget>
269+
</item>
257270
<item>
258271
<widget class="QPushButton" name="btnSaveAs">
259272
<property name="text">

‎src/qt/optionsdialog.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ void OptionsDialog::setModel(OptionsModel *_model)
270270
connect(ui->prune, &QCheckBox::clicked, this, &OptionsDialog::togglePruneWarning);
271271
connect(ui->pruneSize, qOverload<int>(&QSpinBox::valueChanged), this, &OptionsDialog::showRestartWarning);
272272
connect(ui->databaseCache, qOverload<int>(&QSpinBox::valueChanged), this, &OptionsDialog::showRestartWarning);
273+
connect(ui->externalSignerPath, &QLineEdit::textChanged, [this]{ showRestartWarning(); });
273274
connect(ui->threadsScriptVerif, qOverload<int>(&QSpinBox::valueChanged), this, &OptionsDialog::showRestartWarning);
274275
/* Wallet */
275276
connect(ui->showMasternodesTab, &QCheckBox::clicked, this, &OptionsDialog::showRestartWarning);
@@ -336,6 +337,7 @@ void OptionsDialog::setMapper()
336337

337338
/* Wallet */
338339
mapper->addMapping(ui->coinControlFeatures, OptionsModel::CoinControlFeatures);
340+
mapper->addMapping(ui->externalSignerPath, OptionsModel::ExternalSignerPath);
339341
mapper->addMapping(ui->subFeeFromAmount, OptionsModel::SubFeeFromAmount);
340342
mapper->addMapping(ui->m_enable_psbt_controls, OptionsModel::EnablePSBTControls);
341343
mapper->addMapping(ui->keepChangeAddress, OptionsModel::KeepChangeAddress);

‎src/qt/optionsmodel.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,13 @@ void OptionsModel::Init(bool resetSettings)
223223
if (!gArgs.SoftSetBoolArg("-spendzeroconfchange", settings.value("bSpendZeroConfChange").toBool()))
224224
addOverriddenOption("-spendzeroconfchange");
225225

226+
if (!settings.contains("external_signer_path"))
227+
settings.setValue("external_signer_path", "");
228+
229+
if (!gArgs.SoftSetArg("-signer", settings.value("external_signer_path").toString().toStdString())) {
230+
addOverriddenOption("-signer");
231+
}
232+
226233
if (!settings.contains("SubFeeFromAmount")) {
227234
settings.setValue("SubFeeFromAmount", false);
228235
}
@@ -496,6 +503,8 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const
496503
#ifdef ENABLE_WALLET
497504
case SpendZeroConfChange:
498505
return settings.value("bSpendZeroConfChange");
506+
case ExternalSignerPath:
507+
return settings.value("external_signer_path");
499508
case SubFeeFromAmount:
500509
return m_sub_fee_from_amount;
501510
case ShowMasternodesTab:
@@ -671,6 +680,12 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
671680
setRestartRequired(true);
672681
}
673682
break;
683+
case ExternalSignerPath:
684+
if (settings.value("external_signer_path") != value.toString()) {
685+
settings.setValue("external_signer_path", value.toString());
686+
setRestartRequired(true);
687+
}
688+
break;
674689
case ShowMasternodesTab:
675690
if (settings.value("fShowMasternodesTab") != value) {
676691
settings.setValue("fShowMasternodesTab", value);

0 commit comments

Comments
 (0)