Skip to content

Commit c697167

Browse files
achow101achow101
committed
qt: Add a dialog to select the change output when bumping fee
In order to correctly choose the change output when doing fee bumping in the GUI, we need to ask the user which output is change. We can make a guess using our ScriptIsChange heuristic, however the user may have chosen to have a custom change address or have otherwise labeled their change address which makes our change detection fail. By asking the user when fee bumping, we can avoid adding additional change outputs that are unnecessary.
1 parent eacdc14 commit c697167

8 files changed

+272
-3
lines changed

‎build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
<ClCompile Include="..\..\src\qt\bitcoingui.cpp" />
1818
<ClCompile Include="..\..\src\qt\bitcoinstrings.cpp" />
1919
<ClCompile Include="..\..\src\qt\bitcoinunits.cpp" />
20+
<ClCompile Include="..\..\src\qt\bumpfeechoosechangedialog.cpp" />
2021
<ClCompile Include="..\..\src\qt\clientmodel.cpp" />
2122
<ClCompile Include="..\..\src\qt\coincontroldialog.cpp" />
2223
<ClCompile Include="..\..\src\qt\coincontroltreewidget.cpp" />
@@ -73,6 +74,7 @@
7374
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_bitcoinamountfield.cpp" />
7475
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_bitcoingui.cpp" />
7576
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_bitcoinunits.cpp" />
77+
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_bumpfeechoosechangedialog.cpp" />
7678
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_clientmodel.cpp" />
7779
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_coincontroldialog.cpp" />
7880
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_coincontroltreewidget.cpp" />

‎src/Makefile.qt.include

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ include Makefile.qt_locale.include
1616
QT_FORMS_UI = \
1717
qt/forms/addressbookpage.ui \
1818
qt/forms/askpassphrasedialog.ui \
19+
qt/forms/bumpfeechoosechangedialog.ui \
1920
qt/forms/coincontroldialog.ui \
2021
qt/forms/createwalletdialog.ui \
2122
qt/forms/editaddressdialog.ui \
@@ -38,6 +39,7 @@ QT_MOC_CPP = \
3839
qt/moc_addressbookpage.cpp \
3940
qt/moc_addresstablemodel.cpp \
4041
qt/moc_askpassphrasedialog.cpp \
42+
qt/moc_bumpfeechoosechangedialog.cpp \
4143
qt/moc_createwalletdialog.cpp \
4244
qt/moc_bantablemodel.cpp \
4345
qt/moc_bitcoin.cpp \
@@ -109,6 +111,7 @@ BITCOIN_QT_H = \
109111
qt/addressbookpage.h \
110112
qt/addresstablemodel.h \
111113
qt/askpassphrasedialog.h \
114+
qt/bumpfeechoosechangedialog.h \
112115
qt/bantablemodel.h \
113116
qt/bitcoin.h \
114117
qt/bitcoinaddressvalidator.h \
@@ -252,6 +255,7 @@ BITCOIN_QT_WALLET_CPP = \
252255
qt/addressbookpage.cpp \
253256
qt/addresstablemodel.cpp \
254257
qt/askpassphrasedialog.cpp \
258+
qt/bumpfeechoosechangedialog.cpp \
255259
qt/coincontroldialog.cpp \
256260
qt/coincontroltreewidget.cpp \
257261
qt/createwalletdialog.cpp \

‎src/qt/bumpfeechoosechangedialog.cpp

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or https://www.opensource.org/licenses/mit-license.php.
4+
5+
#if defined(HAVE_CONFIG_H)
6+
#include <config/bitcoin-config.h>
7+
#endif
8+
9+
#include <qt/bumpfeechoosechangedialog.h>
10+
#include <qt/forms/ui_bumpfeechoosechangedialog.h>
11+
12+
#include <addresstype.h>
13+
#include <key_io.h>
14+
#include <qt/bitcoinunits.h>
15+
#include <qt/guiutil.h>
16+
#include <qt/optionsmodel.h>
17+
#include <qt/walletmodel.h>
18+
19+
#include <QHBoxLayout>
20+
#include <QLabel>
21+
#include <QRadioButton>
22+
#include <QVBoxLayout>
23+
24+
BumpfeeChooseChangeDialog::BumpfeeChooseChangeDialog(WalletModel *model, QWidget *parent, const uint256& txid) :
25+
QDialog(parent, GUIUtil::dialog_flags),
26+
ui(new Ui::BumpfeeChooseChangeDialog),
27+
model(model)
28+
{
29+
ui->setupUi(this);
30+
31+
bool found_change = false;
32+
CTransactionRef tx = model->wallet().getTx(txid);
33+
for (size_t i = 0; i < tx->vout.size(); ++i) {
34+
const CTxOut& txout = tx->vout.at(i);
35+
QString address_info = tr("No address decoded");
36+
CTxDestination dest;
37+
if (ExtractDestination(txout.scriptPubKey, dest)) {
38+
std::string address = EncodeDestination(dest);
39+
std::string label;
40+
if (model->wallet().getAddress(dest, &label, nullptr, nullptr) && !label.empty()) {
41+
address_info = QString::fromStdString(label) + QString(" (") + QString::fromStdString(address) + QString(")");
42+
} else {
43+
address_info = QString::fromStdString(address);
44+
}
45+
}
46+
QString output_info = QString::number(i) + QString(": ") + BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), txout.nValue) + tr(" to ") + address_info;
47+
48+
QRadioButton *radio_button = new QRadioButton(output_info, nullptr);
49+
radio_button->setObjectName(QString::number(i) + QString("_radioButton"));
50+
ui->verticalLayout->addWidget(radio_button);
51+
52+
if (!found_change && model->wallet().isChange(txout)) {
53+
radio_button->setChecked(true);
54+
ui->none_radioButton->setChecked(false);
55+
found_change = true;
56+
}
57+
}
58+
GUIUtil::handleCloseWindowShortcut(this);
59+
}
60+
61+
std::optional<uint32_t> BumpfeeChooseChangeDialog::GetSelectedOutput()
62+
{
63+
for (int i = 0; i < ui->verticalLayout->count(); ++i) {
64+
QRadioButton* child = dynamic_cast<QRadioButton*>(ui->verticalLayout->itemAt(i)->widget());
65+
if (child->isChecked()) {
66+
if (i == 0) {
67+
// "None" option selected
68+
return std::nullopt;
69+
}
70+
// Return the output index, offset by one for the "None" option at index 0
71+
return static_cast<uint32_t>(i - 1);
72+
}
73+
}
74+
return std::nullopt;
75+
}

‎src/qt/bumpfeechoosechangedialog.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or https://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H
6+
#define BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H
7+
8+
#include <QDialog>
9+
#include <optional>
10+
11+
#include <uint256.h>
12+
13+
class WalletModel;
14+
class uint256;
15+
16+
namespace Ui {
17+
class BumpfeeChooseChangeDialog;
18+
}
19+
20+
/** Dialog for choosing the change output when bumping fee
21+
*/
22+
class BumpfeeChooseChangeDialog : public QDialog
23+
{
24+
Q_OBJECT
25+
26+
public:
27+
28+
explicit BumpfeeChooseChangeDialog(WalletModel *model, QWidget *parent, const uint256& txid);
29+
std::optional<uint32_t> GetSelectedOutput();
30+
31+
private:
32+
Ui::BumpfeeChooseChangeDialog *ui;
33+
WalletModel *model;
34+
};
35+
36+
#endif // BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H

‎src/qt/forms/bumpfeechoosechangedialog.ui

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<ui version="4.0">
3+
<class>BumpfeeChooseChangeDialog</class>
4+
<widget class="QDialog" name="BumpfeeChooseChangeDialog">
5+
<property name="geometry">
6+
<rect>
7+
<x>0</x>
8+
<y>0</y>
9+
<width>737</width>
10+
<height>422</height>
11+
</rect>
12+
</property>
13+
<property name="windowTitle">
14+
<string>Choose Change Output</string>
15+
</property>
16+
<property name="sizeGripEnabled">
17+
<bool>true</bool>
18+
</property>
19+
<layout class="QVBoxLayout" name="verticalLayout_2">
20+
<item>
21+
<widget class="QLabel" name="label">
22+
<property name="text">
23+
<string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;Choose which output is change.&lt;/p&gt;&lt;p&gt;The additional transaction fee will be deducted from this output. If it is insufficient, new inputs may be added and the resulting change sent to the address of the selected output.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
24+
</property>
25+
<property name="textFormat">
26+
<enum>Qt::RichText</enum>
27+
</property>
28+
<property name="wordWrap">
29+
<bool>true</bool>
30+
</property>
31+
</widget>
32+
</item>
33+
<item>
34+
<widget class="QScrollArea" name="scrollArea">
35+
<property name="widgetResizable">
36+
<bool>true</bool>
37+
</property>
38+
<widget class="QWidget" name="scrollAreaWidgetContents">
39+
<property name="geometry">
40+
<rect>
41+
<x>0</x>
42+
<y>0</y>
43+
<width>711</width>
44+
<height>288</height>
45+
</rect>
46+
</property>
47+
<layout class="QVBoxLayout" name="verticalLayout">
48+
<item>
49+
<widget class="QRadioButton" name="none_radioButton">
50+
<property name="text">
51+
<string>None</string>
52+
</property>
53+
<property name="checked">
54+
<bool>true</bool>
55+
</property>
56+
</widget>
57+
</item>
58+
</layout>
59+
</widget>
60+
</widget>
61+
</item>
62+
<item>
63+
<widget class="QDialogButtonBox" name="buttonBox">
64+
<property name="orientation">
65+
<enum>Qt::Horizontal</enum>
66+
</property>
67+
<property name="standardButtons">
68+
<set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
69+
</property>
70+
</widget>
71+
</item>
72+
</layout>
73+
</widget>
74+
<resources/>
75+
<connections>
76+
<connection>
77+
<sender>buttonBox</sender>
78+
<signal>accepted()</signal>
79+
<receiver>BumpfeeChooseChangeDialog</receiver>
80+
<slot>accept()</slot>
81+
<hints>
82+
<hint type="sourcelabel">
83+
<x>210</x>
84+
<y>395</y>
85+
</hint>
86+
<hint type="destinationlabel">
87+
<x>200</x>
88+
<y>210</y>
89+
</hint>
90+
</hints>
91+
</connection>
92+
<connection>
93+
<sender>buttonBox</sender>
94+
<signal>rejected()</signal>
95+
<receiver>BumpfeeChooseChangeDialog</receiver>
96+
<slot>reject()</slot>
97+
<hints>
98+
<hint type="sourcelabel">
99+
<x>210</x>
100+
<y>395</y>
101+
</hint>
102+
<hint type="destinationlabel">
103+
<x>200</x>
104+
<y>210</y>
105+
</hint>
106+
</hints>
107+
</connection>
108+
</connections>
109+
</ui>

‎src/qt/test/wallettests.cpp

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <key_io.h>
1212
#include <qt/bitcoinamountfield.h>
1313
#include <qt/bitcoinunits.h>
14+
#include <qt/bumpfeechoosechangedialog.h>
1415
#include <qt/clientmodel.h>
1516
#include <qt/optionsmodel.h>
1617
#include <qt/overviewpage.h>
@@ -40,6 +41,7 @@
4041
#include <QClipboard>
4142
#include <QObject>
4243
#include <QPushButton>
44+
#include <QRadioButton>
4345
#include <QTimer>
4446
#include <QVBoxLayout>
4547
#include <QTextEdit>
@@ -61,7 +63,7 @@ namespace
6163
//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
6264
void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
6365
{
64-
QTimer::singleShot(0, [text, confirm_type]() {
66+
QTimer::singleShot(10ms, [text, confirm_type]() {
6567
for (QWidget* widget : QApplication::topLevelWidgets()) {
6668
if (widget->inherits("SendConfirmationDialog")) {
6769
SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
@@ -74,6 +76,35 @@ void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_ty
7476
});
7577
}
7678

79+
//! In the BumpfeeChooseChangeDialog, choose the radio button at the index, or use the default. Then Press "Yes" or "Cancel".
80+
void ChooseBumpfeeOutput(std::optional<uint32_t> index = std::nullopt, bool cancel = false)
81+
{
82+
QTimer::singleShot(0, [index, cancel]() {
83+
for (QWidget* widget : QApplication::topLevelWidgets()) {
84+
if (widget->inherits("BumpfeeChooseChangeDialog")) {
85+
BumpfeeChooseChangeDialog* dialog = qobject_cast<BumpfeeChooseChangeDialog*>(widget);
86+
87+
if (index.has_value()) {
88+
QString button_name;
89+
if (index.value() == 0) {
90+
button_name = QString("none_radioButton");
91+
} else {
92+
button_name = QString::number(index.value() - 1) + QString("_radioButton");
93+
}
94+
QRadioButton* button = dialog->findChild<QRadioButton*>(button_name);
95+
button->setEnabled(true);
96+
button->click();
97+
}
98+
99+
QDialogButtonBox* button_box = dialog->findChild<QDialogButtonBox*>(QString("buttonBox"));
100+
QAbstractButton* button = button_box->button(cancel ? QDialogButtonBox::Cancel : QDialogButtonBox::Ok);
101+
button->setEnabled(true);
102+
button->click();
103+
}
104+
}
105+
});
106+
}
107+
77108
//! Send coins to address and return txid.
78109
uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
79110
QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
@@ -126,11 +157,12 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st
126157
QCOMPARE(action->isEnabled(), !expectDisabled);
127158

128159
action->setEnabled(true);
160+
ChooseBumpfeeOutput();
129161
QString text;
130162
if (expectError.empty()) {
131163
ConfirmSend(&text, cancel ? QMessageBox::Cancel : QMessageBox::Yes);
132164
} else {
133-
ConfirmMessage(&text, 0ms);
165+
ConfirmMessage(&text, 10ms);
134166
}
135167
action->trigger();
136168
QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1);

‎src/qt/walletmodel.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <qt/walletmodel.h>
1010

1111
#include <qt/addresstablemodel.h>
12+
#include <qt/bumpfeechoosechangedialog.h>
1213
#include <qt/clientmodel.h>
1314
#include <qt/guiconstants.h>
1415
#include <qt/guiutil.h>
@@ -32,6 +33,7 @@
3233
#include <functional>
3334

3435
#include <QDebug>
36+
#include <QDialog>
3537
#include <QMessageBox>
3638
#include <QSet>
3739
#include <QTimer>
@@ -477,13 +479,21 @@ WalletModel::UnlockContext::~UnlockContext()
477479

478480
bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
479481
{
482+
// Ask the user which is the change output
483+
auto choose_change_dialog = new BumpfeeChooseChangeDialog(this, nullptr, hash);
484+
const auto choose_change_retval = choose_change_dialog->exec();
485+
if (choose_change_retval != QDialog::Accepted) {
486+
return false;
487+
}
488+
std::optional<uint32_t> change_pos = choose_change_dialog->GetSelectedOutput();
489+
480490
CCoinControl coin_control;
481491
coin_control.m_signal_bip125_rbf = true;
482492
std::vector<bilingual_str> errors;
483493
CAmount old_fee;
484494
CAmount new_fee;
485495
CMutableTransaction mtx;
486-
if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
496+
if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx, change_pos)) {
487497
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
488498
(errors.size() ? QString::fromStdString(errors[0].translated) : "") +")");
489499
return false;

‎test/lint/lint-circular-dependencies.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel",
2020
"qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog",
2121
"qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel",
22+
"qt/bumpfeechoosechangedialog -> qt/walletmodel -> qt/bumpfeechoosechangedialog",
2223
"wallet/wallet -> wallet/walletdb -> wallet/wallet",
2324
"kernel/coinstats -> validation -> kernel/coinstats",
2425

0 commit comments

Comments
 (0)