Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 6547892

Browse files
achow101achow101
committedMay 19, 2023
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 2640948 commit 6547892

8 files changed

+272
-3
lines changed
 

‎build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj

+2
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

+4
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

+75
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

+36
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

+109
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

+34-2
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>
@@ -38,6 +39,7 @@
3839
#include <QClipboard>
3940
#include <QObject>
4041
#include <QPushButton>
42+
#include <QRadioButton>
4143
#include <QTimer>
4244
#include <QVBoxLayout>
4345
#include <QTextEdit>
@@ -59,7 +61,7 @@ namespace
5961
//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
6062
void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
6163
{
62-
QTimer::singleShot(0, [text, confirm_type]() {
64+
QTimer::singleShot(10ms, [text, confirm_type]() {
6365
for (QWidget* widget : QApplication::topLevelWidgets()) {
6466
if (widget->inherits("SendConfirmationDialog")) {
6567
SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
@@ -72,6 +74,35 @@ void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_ty
7274
});
7375
}
7476

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

126157
action->setEnabled(true);
158+
ChooseBumpfeeOutput();
127159
QString text;
128160
if (expectError.empty()) {
129161
ConfirmSend(&text, cancel ? QMessageBox::Cancel : QMessageBox::Yes);
130162
} else {
131-
ConfirmMessage(&text, 0ms);
163+
ConfirmMessage(&text, 10ms);
132164
}
133165
action->trigger();
134166
QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1);

‎src/qt/walletmodel.cpp

+11-1
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>
@@ -478,13 +480,21 @@ WalletModel::UnlockContext::~UnlockContext()
478480

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

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

+1
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/fees -> wallet/wallet -> wallet/fees",
2324
"wallet/wallet -> wallet/walletdb -> wallet/wallet",
2425
"kernel/coinstats -> validation -> kernel/coinstats",

0 commit comments

Comments
 (0)
Please sign in to comment.