Skip to content

Commit bf222ab

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 b1b9278 commit bf222ab

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 <key_io.h>
13+
#include <script/standard.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
@@ -10,6 +10,7 @@
1010
#include <key_io.h>
1111
#include <qt/bitcoinamountfield.h>
1212
#include <qt/bitcoinunits.h>
13+
#include <qt/bumpfeechoosechangedialog.h>
1314
#include <qt/clientmodel.h>
1415
#include <qt/optionsmodel.h>
1516
#include <qt/overviewpage.h>
@@ -35,6 +36,7 @@
3536
#include <QApplication>
3637
#include <QCheckBox>
3738
#include <QPushButton>
39+
#include <QRadioButton>
3840
#include <QTimer>
3941
#include <QVBoxLayout>
4042
#include <QTextEdit>
@@ -55,7 +57,7 @@ namespace
5557
//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
5658
void ConfirmSend(QString* text = nullptr, bool cancel = false)
5759
{
58-
QTimer::singleShot(0, [text, cancel]() {
60+
QTimer::singleShot(10ms, [text, cancel]() {
5961
for (QWidget* widget : QApplication::topLevelWidgets()) {
6062
if (widget->inherits("SendConfirmationDialog")) {
6163
SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
@@ -68,6 +70,35 @@ void ConfirmSend(QString* text = nullptr, bool cancel = false)
6870
});
6971
}
7072

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

121152
action->setEnabled(true);
153+
ChooseBumpfeeOutput();
122154
QString text;
123155
if (expectError.empty()) {
124156
ConfirmSend(&text, cancel);
125157
} else {
126-
ConfirmMessage(&text, 0ms);
158+
ConfirmMessage(&text, 10ms);
127159
}
128160
action->trigger();
129161
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>
@@ -485,13 +487,21 @@ void WalletModel::UnlockContext::CopyFrom(UnlockContext&& rhs)
485487

486488
bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
487489
{
490+
// Ask the user which is the change output
491+
auto choose_change_dialog = new BumpfeeChooseChangeDialog(this, nullptr, hash);
492+
const auto choose_change_retval = choose_change_dialog->exec();
493+
if (choose_change_retval != QDialog::Accepted) {
494+
return false;
495+
}
496+
std::optional<uint32_t> change_pos = choose_change_dialog->GetSelectedOutput();
497+
488498
CCoinControl coin_control;
489499
coin_control.m_signal_bip125_rbf = true;
490500
std::vector<bilingual_str> errors;
491501
CAmount old_fee;
492502
CAmount new_fee;
493503
CMutableTransaction mtx;
494-
if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
504+
if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx, change_pos)) {
495505
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
496506
(errors.size() ? QString::fromStdString(errors[0].translated) : "") +")");
497507
return false;

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

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

0 commit comments

Comments
 (0)