Dialog for allowing the user to choose the change output when bumping a tx

src/interfaces/wallet.h

@@ -131,6 +131,9 @@ class Wallet
     //! Save or remove receive request.
     virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0;
 
+    //! Whether the given output is a change
+    virtual bool isChange(const CTxOut& txout) const = 0;
+
     //! Display address on external signer
     virtual util::Result<void> displayAddress(const CTxDestination& dest) = 0;
 
@@ -173,7 +176,8 @@ class Wallet
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
         CAmount& new_fee,
-        CMutableTransaction& mtx) = 0;
+        CMutableTransaction& mtx,
+        std::optional<uint32_t> reduce_output) = 0;
 
     //! Sign bump transaction.
     virtual bool signBumpTransaction(CMutableTransaction& mtx) = 0;

src/qt/CMakeLists.txt

@@ -63,6 +63,8 @@ add_library(bitcoinqt STATIC EXCLUDE_FROM_ALL
   bitcoingui.h
   bitcoinunits.cpp
   bitcoinunits.h
+  bumpfeechoosechangedialog.cpp
+  bumpfeechoosechangedialog.h
   clientmodel.cpp
   clientmodel.h
   csvmodelwriter.cpp

src/qt/bumpfeechoosechangedialog.cpp

@@ -0,0 +1,75 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#if defined(HAVE_CONFIG_H)
+#include <config/bitcoin-config.h>
+#endif
+
+#include <qt/bumpfeechoosechangedialog.h>
+#include <qt/forms/ui_bumpfeechoosechangedialog.h>
+
+#include <addresstype.h>
+#include <key_io.h>
+#include <qt/bitcoinunits.h>
+#include <qt/guiutil.h>
+#include <qt/optionsmodel.h>
+#include <qt/walletmodel.h>
+
+#include <QHBoxLayout>
+#include <QLabel>
+#include <QRadioButton>
+#include <QVBoxLayout>
+
+BumpfeeChooseChangeDialog::BumpfeeChooseChangeDialog(WalletModel *model, QWidget *parent, const uint256& txid) :
+    QDialog(parent, GUIUtil::dialog_flags),
+    ui(new Ui::BumpfeeChooseChangeDialog),
+    model(model)
+{
+    ui->setupUi(this);
+
+    bool found_change = false;
+    CTransactionRef tx = model->wallet().getTx(txid);
+    for (size_t i = 0; i < tx->vout.size(); ++i) {
+        const CTxOut& txout = tx->vout.at(i);
+        QString address_info = tr("No address decoded");
+        CTxDestination dest;
+        if (ExtractDestination(txout.scriptPubKey, dest)) {
+            std::string address = EncodeDestination(dest);
+            std::string label;
+            if (model->wallet().getAddress(dest, &label, nullptr, nullptr) && !label.empty()) {
+                address_info = QString::fromStdString(label) + QString(" (") + QString::fromStdString(address) + QString(")");
+            } else {
+                address_info = QString::fromStdString(address);
+            }
+        }
+        QString output_info = tr("%1: %2 to %3").arg(i).arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), txout.nValue)).arg(address_info);
+
+        QRadioButton *radio_button = new QRadioButton(output_info, nullptr);
+        radio_button->setObjectName(QString::number(i) + QString("_radioButton"));
+        ui->verticalLayout->addWidget(radio_button);
+
+        if (!found_change && model->wallet().isChange(txout)) {
+            radio_button->setChecked(true);
+            ui->none_radioButton->setChecked(false);
+            found_change = true;
+        }
+    }
+    GUIUtil::handleCloseWindowShortcut(this);
+}
+
+std::optional<uint32_t> BumpfeeChooseChangeDialog::GetSelectedOutput()
+{
+    for (int i = 0; i < ui->verticalLayout->count(); ++i) {
+        QRadioButton* child = dynamic_cast<QRadioButton*>(ui->verticalLayout->itemAt(i)->widget());
+        if (child->isChecked()) {
+            if (i == 0) {
+                // "None" option selected
+                return std::nullopt;
+            }
+            // Return the output index, offset by one for the "None" option at index 0
+            return static_cast<uint32_t>(i - 1);
+        }
+    }
+    return std::nullopt;
+}

src/qt/bumpfeechoosechangedialog.h

@@ -0,0 +1,36 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H
+#define BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H
+
+#include <QDialog>
+#include <optional>
+
+#include <uint256.h>
+
+class WalletModel;
+class uint256;
+
+namespace Ui {
+    class BumpfeeChooseChangeDialog;
+}
+
+/** Dialog for choosing the change output when bumping fee
+ */
+class BumpfeeChooseChangeDialog : public QDialog
+{
+    Q_OBJECT
+
+public:
+
+    explicit BumpfeeChooseChangeDialog(WalletModel *model, QWidget *parent, const uint256& txid);
+    std::optional<uint32_t> GetSelectedOutput();
+
+private:
+    Ui::BumpfeeChooseChangeDialog *ui;
+    WalletModel *model;
+};
+
+#endif // BITCOIN_QT_BUMPFEECHOOSECHANGEDIALOG_H

src/qt/forms/bumpfeechoosechangedialog.ui

@@ -0,0 +1,109 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>BumpfeeChooseChangeDialog</class>
+ <widget class="QDialog" name="BumpfeeChooseChangeDialog">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>737</width>
+    <height>422</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>Choose Change Output</string>
+  </property>
+  <property name="sizeGripEnabled">
+   <bool>true</bool>
+  </property>
+  <layout class="QVBoxLayout" name="verticalLayout_2">
+   <item>
+    <widget class="QLabel" name="label">
+     <property name="text">
+      <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>
+     </property>
+     <property name="textFormat">
+      <enum>Qt::RichText</enum>
+     </property>
+     <property name="wordWrap">
+      <bool>true</bool>
+     </property>
+    </widget>
+   </item>
+   <item>
+    <widget class="QScrollArea" name="scrollArea">
+     <property name="widgetResizable">
+      <bool>true</bool>
+     </property>
+     <widget class="QWidget" name="scrollAreaWidgetContents">
+      <property name="geometry">
+       <rect>
+        <x>0</x>
+        <y>0</y>
+        <width>711</width>
+        <height>288</height>
+       </rect>
+      </property>
+      <layout class="QVBoxLayout" name="verticalLayout">
+       <item>
+        <widget class="QRadioButton" name="none_radioButton">
+         <property name="text">
+          <string>None</string>
+         </property>
+         <property name="checked">
+          <bool>true</bool>
+         </property>
+        </widget>
+       </item>
+      </layout>
+     </widget>
+    </widget>
+   </item>
+   <item>
+    <widget class="QDialogButtonBox" name="buttonBox">
+     <property name="orientation">
+      <enum>Qt::Horizontal</enum>
+     </property>
+     <property name="standardButtons">
+      <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
+     </property>
+    </widget>
+   </item>
+  </layout>
+ </widget>
+ <resources/>
+ <connections>
+  <connection>
+   <sender>buttonBox</sender>
+   <signal>accepted()</signal>
+   <receiver>BumpfeeChooseChangeDialog</receiver>
+   <slot>accept()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>210</x>
+     <y>395</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>200</x>
+     <y>210</y>
+    </hint>
+   </hints>
+  </connection>
+  <connection>
+   <sender>buttonBox</sender>
+   <signal>rejected()</signal>
+   <receiver>BumpfeeChooseChangeDialog</receiver>
+   <slot>reject()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>210</x>
+     <y>395</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>200</x>
+     <y>210</y>
+    </hint>
+   </hints>
+  </connection>
+ </connections>
+</ui>

src/qt/test/wallettests.cpp

@@ -11,6 +11,7 @@
 #include <key_io.h>
 #include <qt/bitcoinamountfield.h>
 #include <qt/bitcoinunits.h>
+#include <qt/bumpfeechoosechangedialog.h>
 #include <qt/clientmodel.h>
 #include <qt/optionsmodel.h>
 #include <qt/overviewpage.h>
@@ -40,6 +41,7 @@
 #include <QClipboard>
 #include <QObject>
 #include <QPushButton>
+#include <QRadioButton>
 #include <QTimer>
 #include <QVBoxLayout>
 #include <QTextEdit>
@@ -61,7 +63,7 @@ namespace
 //! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
 void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
 {
-    QTimer::singleShot(0, [text, confirm_type]() {
+    QTimer::singleShot(10ms, [text, confirm_type]() {
         for (QWidget* widget : QApplication::topLevelWidgets()) {
             if (widget->inherits("SendConfirmationDialog")) {
                 SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
@@ -74,6 +76,35 @@ void ConfirmSend(QString* text = nullptr, QMessageBox::StandardButton confirm_ty
     });
 }
 
+//! In the BumpfeeChooseChangeDialog, choose the radio button at the index, or use the default. Then Press "Yes" or "Cancel".
+void ChooseBumpfeeOutput(std::optional<uint32_t> index = std::nullopt, bool cancel = false)
+{
+    QTimer::singleShot(0, [index, cancel]() {
+        for (QWidget* widget : QApplication::topLevelWidgets()) {
+            if (widget->inherits("BumpfeeChooseChangeDialog")) {
+                BumpfeeChooseChangeDialog* dialog = qobject_cast<BumpfeeChooseChangeDialog*>(widget);
+
+                if (index.has_value()) {
+                    QString button_name;
+                    if (index.value() == 0) {
+                        button_name = QString("none_radioButton");
+                    } else {
+                        button_name = QString::number(index.value() - 1) + QString("_radioButton");
+                    }
+                    QRadioButton* button = dialog->findChild<QRadioButton*>(button_name);
+                    button->setEnabled(true);
+                    button->click();
+                }
+
+                QDialogButtonBox* button_box = dialog->findChild<QDialogButtonBox*>(QString("buttonBox"));
+                QAbstractButton* button = button_box->button(cancel ? QDialogButtonBox::Cancel : QDialogButtonBox::Ok);
+                button->setEnabled(true);
+                button->click();
+            }
+        }
+    });
+}
+
 //! Send coins to address and return txid.
 uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf,
                   QMessageBox::StandardButton confirm_type = QMessageBox::Yes)
@@ -126,11 +157,12 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st
     QCOMPARE(action->isEnabled(), !expectDisabled);
 
     action->setEnabled(true);
+    ChooseBumpfeeOutput();
     QString text;
     if (expectError.empty()) {
         ConfirmSend(&text, cancel ? QMessageBox::Cancel : QMessageBox::Yes);
     } else {
-        ConfirmMessage(&text, 0ms);
+        ConfirmMessage(&text, 10ms);
     }
     action->trigger();
     QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1);

src/qt/walletmodel.cpp

@@ -5,6 +5,7 @@
 #include <qt/walletmodel.h>
 
 #include <qt/addresstablemodel.h>
+#include <qt/bumpfeechoosechangedialog.h>
 #include <qt/clientmodel.h>
 #include <qt/guiconstants.h>
 #include <qt/guiutil.h>
@@ -29,6 +30,7 @@
 #include <functional>
 
 #include <QDebug>
+#include <QDialog>
 #include <QMessageBox>
 #include <QSet>
 #include <QTimer>
@@ -481,13 +483,21 @@ WalletModel::UnlockContext::~UnlockContext()
 
 bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
 {
+    // Ask the user which is the change output
+    auto choose_change_dialog = new BumpfeeChooseChangeDialog(this, nullptr, hash);
+    const auto choose_change_retval = choose_change_dialog->exec();
+    if (choose_change_retval != QDialog::Accepted) {
+        return false;
+    }
+    std::optional<uint32_t> change_pos = choose_change_dialog->GetSelectedOutput();
+
     CCoinControl coin_control;
     coin_control.m_signal_bip125_rbf = true;
     std::vector<bilingual_str> errors;
     CAmount old_fee;
     CAmount new_fee;
     CMutableTransaction mtx;
-    if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx)) {
+    if (!m_wallet->createBumpTransaction(hash, coin_control, errors, old_fee, new_fee, mtx, change_pos)) {
         QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
             (errors.size() ? QString::fromStdString(errors[0].translated) : "") +")");
         return false;

src/wallet/interfaces.cpp

@@ -249,6 +249,11 @@ class WalletImpl : public Wallet
         return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id)
                              : m_wallet->SetAddressReceiveRequest(batch, dest, id, value);
     }
+    bool isChange(const CTxOut& txout) const override
+    {
+        LOCK(m_wallet->cs_wallet);
+        return OutputIsChange(*m_wallet, txout);
+    }
     util::Result<void> displayAddress(const CTxDestination& dest) override
     {
         LOCK(m_wallet->cs_wallet);
@@ -314,10 +319,11 @@ class WalletImpl : public Wallet
         std::vector<bilingual_str>& errors,
         CAmount& old_fee,
         CAmount& new_fee,
-        CMutableTransaction& mtx) override
+        CMutableTransaction& mtx,
+        std::optional<uint32_t> reduce_output) override
     {
         std::vector<CTxOut> outputs; // just an empty list of new recipients for now
-        return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs) == feebumper::Result::OK;
+        return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs, reduce_output) == feebumper::Result::OK;
     }
     bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
     bool commitBumpTransaction(const uint256& txid,

test/lint/lint-circular-dependencies.py

@@ -19,6 +19,7 @@
     "qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel",
     "qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog",
     "qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel",
+    "qt/bumpfeechoosechangedialog -> qt/walletmodel -> qt/bumpfeechoosechangedialog",
     "wallet/wallet -> wallet/walletdb -> wallet/wallet",
     "kernel/coinstats -> validation -> kernel/coinstats",