Merge bitcoin-core/gui#441: Add Create Unsigned button to SendConfirmationDialog

BACKPORT NOTE:
Formatting in src/qt/optionsmodel.h has been changed in bitcoin#3267.
These changes has not been backported on time; done with this backport

742918c qt: hide Create Unsigned button behind an expert mode option (Andrew Chow)
5c3b800 qt: Add Create Unsigned button to SendConfirmationDialog (Andrew Chow)

Pull request description:

  Instead of having different buttons or changing button behavior for making a PSBT, just have SendConfirmationDialog return whether the user wants a PSBT or a broadcasted transaction. Since this dialog is used by both the bumpFeeAction and the SendCoinsDialog, changes to both to support the different behavior is needed. They will check the return value of the SendConfirmationDialog for whether a PSBT needs to be created instead of checking whether private keys are disabled.

  Strings used in this dialog are being slightly modified to work with both private keys enabled and disabled wallets.

  Moved from bitcoin#18789

ACKs for top commit:
  jarolrod:
    ACK 742918c
  ryanofsky:
    Code review ACK 742918c. Just suggested changes since last review. Looks great!
  hebasto:
    ACK 742918c, tested on Linux Mint 20.2 (Qt 5.12.8).

Tree-SHA512: dd29f4364c7b4f15befe8fe63257b26187918786b005e0f8336183270b1a162680b93f6ced60f0285c6e607c084cc0d24950fc68a8f9c056521ede614041be66

Author: hebasto

src/qt/forms/optionsdialog.ui

@@ -347,6 +347,16 @@
            </property>
           </widget>
          </item>
+         <item>
+          <widget class="QCheckBox" name="m_enable_psbt_controls">
+           <property name="text">
+            <string extracomment="An options window setting to enable PSBT controls.">Enable &amp;PSBT controls</string>
+           </property>
+           <property name="toolTip">
+            <string extracomment="Tooltip text for options window setting that enables PSBT controls.">Whether to show PSBT controls.</string>
+           </property>
+          </widget>
+         </item>
          <item>
           <widget class="QCheckBox" name="keepChangeAddress">
            <property name="toolTip">

src/qt/optionsdialog.cpp

@@ -329,6 +329,7 @@ void OptionsDialog::setMapper()
     /* Wallet */
     mapper->addMapping(ui->coinControlFeatures, OptionsModel::CoinControlFeatures);
     mapper->addMapping(ui->subFeeFromAmount, OptionsModel::SubFeeFromAmount);
+    mapper->addMapping(ui->m_enable_psbt_controls, OptionsModel::EnablePSBTControls);
     mapper->addMapping(ui->keepChangeAddress, OptionsModel::KeepChangeAddress);
     mapper->addMapping(ui->showMasternodesTab, OptionsModel::ShowMasternodesTab);
     mapper->addMapping(ui->showGovernanceTab, OptionsModel::ShowGovernanceTab);

src/qt/optionsmodel.cpp

@@ -157,6 +157,11 @@ void OptionsModel::Init(bool resetSettings)
         settings.setValue("fCoinControlFeatures", false);
     fCoinControlFeatures = settings.value("fCoinControlFeatures", false).toBool();
 
+    if (!settings.contains("enable_psbt_controls")) {
+        settings.setValue("enable_psbt_controls", false);
+    }
+    m_enable_psbt_controls = settings.value("enable_psbt_controls", false).toBool();
+
     if (!settings.contains("fKeepChangeAddress"))
         settings.setValue("fKeepChangeAddress", false);
     fKeepChangeAddress = settings.value("fKeepChangeAddress", false).toBool();
@@ -551,6 +556,8 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const
 #ifdef ENABLE_WALLET
         case CoinControlFeatures:
             return fCoinControlFeatures;
+        case EnablePSBTControls:
+            return settings.value("enable_psbt_controls");
         case KeepChangeAddress:
             return fKeepChangeAddress;
 #endif // ENABLE_WALLET
@@ -800,6 +807,10 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
             settings.setValue("fCoinControlFeatures", fCoinControlFeatures);
             Q_EMIT coinControlFeaturesChanged(fCoinControlFeatures);
             break;
+        case EnablePSBTControls:
+            m_enable_psbt_controls = value.toBool();
+            settings.setValue("enable_psbt_controls", m_enable_psbt_controls);
+            break;
         case KeepChangeAddress:
             fKeepChangeAddress = value.toBool();
             settings.setValue("fKeepChangeAddress", fKeepChangeAddress);

src/qt/optionsmodel.h

@@ -45,49 +45,50 @@ class OptionsModel : public QAbstractListModel
     explicit OptionsModel(QObject *parent = nullptr, bool resetSettings = false);
 
     enum OptionID {
-        StartAtStartup,       // bool
-        ShowTrayIcon,         // bool
-        MinimizeToTray,       // bool
-        MapPortUPnP,          // bool
-        MapPortNatpmp,        // bool
-        MinimizeOnClose,      // bool
-        ProxyUse,             // bool
-        ProxyIP,              // QString
-        ProxyPort,            // int
-        ProxyUseTor,          // bool
-        ProxyIPTor,           // QString
-        ProxyPortTor,         // int
-        DisplayUnit,          // BitcoinUnit
-        ThirdPartyTxUrls,     // QString
-        Digits,               // QString
-        Theme,                // QString
-        FontFamily,           // int
-        FontScale,            // int
-        FontWeightNormal,     // int
-        FontWeightBold,       // int
-        Language,             // QString
-        CoinControlFeatures,  // bool
-        SubFeeFromAmount,     // bool
-        KeepChangeAddress,    // bool
-        ThreadsScriptVerif,   // int
-        Prune,                // bool
-        PruneSize,            // int
-        DatabaseCache,        // int
-        SpendZeroConfChange,  // bool
-        ShowMasternodesTab,   // bool
-        ShowGovernanceTab,    // bool
-        CoinJoinEnabled,      // bool
-        ShowAdvancedCJUI,     // bool
-        ShowCoinJoinPopups,   // bool
-        LowKeysWarning,       // bool
-        CoinJoinSessions,     // int
-        CoinJoinRounds,       // int
-        CoinJoinAmount,       // int
-        CoinJoinDenomsGoal,   // int
-        CoinJoinDenomsHardCap,// int
-        CoinJoinMultiSession, // bool
-        Listen,               // bool
-        Server,               // bool
+        StartAtStartup,         // bool
+        ShowTrayIcon,           // bool
+        MinimizeToTray,         // bool
+        MapPortUPnP,            // bool
+        MapPortNatpmp,          // bool
+        MinimizeOnClose,        // bool
+        ProxyUse,               // bool
+        ProxyIP,                // QString
+        ProxyPort,              // int
+        ProxyUseTor,            // bool
+        ProxyIPTor,             // QString
+        ProxyPortTor,           // int
+        DisplayUnit,            // BitcoinUnit
+        ThirdPartyTxUrls,       // QString
+        Digits,                 // QString
+        Theme,                  // QString
+        FontFamily,             // int
+        FontScale,              // int
+        FontWeightNormal,       // int
+        FontWeightBold,         // int
+        Language,               // QString
+        CoinControlFeatures,    // bool
+        SubFeeFromAmount,       // bool
+        KeepChangeAddress,      // bool
+        ThreadsScriptVerif,     // int
+        Prune,                  // bool
+        PruneSize,              // int
+        DatabaseCache,          // int
+        SpendZeroConfChange,    // bool
+        ShowMasternodesTab,     // bool
+        ShowGovernanceTab,      // bool
+        CoinJoinEnabled,        // bool
+        ShowAdvancedCJUI,       // bool
+        ShowCoinJoinPopups,     // bool
+        LowKeysWarning,         // bool
+        CoinJoinSessions,       // int
+        CoinJoinRounds,         // int
+        CoinJoinAmount,         // int
+        CoinJoinDenomsGoal,     // int
+        CoinJoinDenomsHardCap,  // int
+        CoinJoinMultiSession,   // bool
+        Listen,                 // bool
+        Server,                 // bool
+        EnablePSBTControls,     // bool
         OptionIDRowCount,
     };
 
@@ -108,6 +109,7 @@ class OptionsModel : public QAbstractListModel
     QString getThirdPartyTxUrls() const { return strThirdPartyTxUrls; }
     bool getCoinControlFeatures() const { return fCoinControlFeatures; }
     bool getSubFeeFromAmount() const { return m_sub_fee_from_amount; }
+    bool getEnablePSBTControls() const { return m_enable_psbt_controls; }
     bool getKeepChangeAddress() const { return fKeepChangeAddress; }
     bool getShowAdvancedCJUI() { return fShowAdvancedCJUI; }
     const QString& getOverriddenByCommandLine() { return strOverriddenByCommandLine; }
@@ -136,6 +138,7 @@ class OptionsModel : public QAbstractListModel
     QString strThirdPartyTxUrls;
     bool fCoinControlFeatures;
     bool m_sub_fee_from_amount;
+    bool m_enable_psbt_controls;
     bool fKeepChangeAddress;
     bool fShowAdvancedCJUI;
     /* settings that were overridden by command-line */

src/qt/sendcoinsdialog.cpp

@@ -42,8 +42,6 @@ using wallet::DEFAULT_PAY_TX_FEE;
 #include <QSettings>
 #include <QTextDocument>
 
-#define SEND_CONFIRM_DELAY   3
-
 static constexpr std::array confTargets{2, 4, 6, 12, 24, 48, 144, 504, 1008};
 int getConfTargetForIndex(int index) {
     if (index+1 > static_cast<int>(confTargets.size())) {
@@ -366,15 +364,26 @@ bool SendCoinsDialog::send(const QList<SendCoinsRecipient>& recipients, QString&
         formatted_short.erase(formatted_short.begin() + MAX_SEND_POPUP_ENTRIES, formatted_short.end());
     }
 
-    if (model->wallet().privateKeysDisabled()) {
-        question_string.append(tr("Do you want to draft this transaction?"));
+    /*: Message displayed when attempting to create a transaction. Cautionary text to prompt the user to verify
+        that the displayed transaction details represent the transaction the user intends to create. */
+    question_string.append(tr("Do you want to create this transaction?"));
+    question_string.append("<br /><span style='font-size:10pt;'>");
+    // TODO: re-enable it when external signer will be backported
+    // if (model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner()) {
+    const bool external_signer_available{false};
+    if (external_signer_available) {
+        /*: Text to inform a user attempting to create a transaction of their current options. At this stage,
+            a user can only create a PSBT. This string is displayed when private keys are disabled and an external
+            signer is not available. */
+        question_string.append(tr("Please, review your transaction proposal. This will produce a Partially Signed Bitcoin Transaction (PSBT) which you can save or copy and then sign with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
+    } else if (model->getOptionsModel()->getEnablePSBTControls()) {
+        /*: Text to inform a user attempting to create a transaction of their current options. At this stage,
+            a user can send their transaction or create a PSBT. This string is displayed when both private keys
+            and PSBT controls are enabled. */
+        question_string.append(tr("Please, review your transaction. You can create and send this transaction or create a Partially Signed Bitcoin Transaction (PSBT), which you can save or copy and then sign with, e.g., an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
     } else {
-        question_string.append(tr("Are you sure you want to send?"));
-    }
-    if (model->wallet().privateKeysDisabled()) {
-        question_string.append("<br /><span style='font-size:10pt;'>");
-        question_string.append(tr("This will produce a Partially Signed Transaction (PSBT) which you can save or copy and then sign with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
-        question_string.append("</span>");
+        /*: Text to prompt a user to review the details of the transaction they are attempting to send. */
+        question_string.append(tr("Please, review your transaction."));
     }
     question_string.append("<br /><br />");
     question_string.append(formatted_short.join("<br />"));
@@ -472,21 +481,20 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
     if (!PrepareSendText(question_string, informative_text, detailed_text)) return;
     assert(m_current_transaction);
 
-    const QString confirmation = model->wallet().privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins");
-    const QString confirmButtonText = model->wallet().privateKeysDisabled() ? tr("Create Unsigned") : tr("Send");
-    auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this);
+    const QString confirmation = tr("Confirm send coins");
+    auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, !model->wallet().privateKeysDisabled(), model->getOptionsModel()->getEnablePSBTControls(), this);
     confirmationDialog->setAttribute(Qt::WA_DeleteOnClose);
     // TODO: Replace QDialog::exec() with safer QDialog::show().
     const auto retval = static_cast<QMessageBox::StandardButton>(confirmationDialog->exec());
 
-    if(retval != QMessageBox::Yes)
+    if(retval != QMessageBox::Yes && retval != QMessageBox::Save)
     {
         fNewRecipientAllowed = true;
         return;
     }
 
     bool send_failure = false;
-    if (model->wallet().privateKeysDisabled()) {
+    if (retval == QMessageBox::Save) {
         CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
         PartiallySignedTransaction psbtx(mtx);
         bool complete = false;
@@ -536,6 +544,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
             assert(false);
         }
     } else {
+        assert(!model->wallet().privateKeysDisabled());
         // now send the prepared transaction
         model->sendCoins(*m_current_transaction, m_coin_control->IsUsingCoinJoin());
         Q_EMIT coinsSent(m_current_transaction->getWtx()->GetHash());
@@ -1050,8 +1059,8 @@ void SendCoinsDialog::keepChangeAddressChanged(bool checked)
     fKeepChangeAddress = checked;
 }
 
-SendConfirmationDialog::SendConfirmationDialog(const QString& title, const QString& text, const QString& informative_text, const QString& detailed_text, int _secDelay, const QString& _confirmButtonText, QWidget* parent)
-    : QMessageBox(parent), secDelay(_secDelay), confirmButtonText(_confirmButtonText)
+SendConfirmationDialog::SendConfirmationDialog(const QString& title, const QString& text, const QString& informative_text, const QString& detailed_text, int _secDelay, bool enable_send, bool always_show_unsigned, QWidget* parent)
+    : QMessageBox(parent), secDelay(_secDelay), m_enable_send(enable_send)
 {
     GUIUtil::updateFonts();
     setIcon(QMessageBox::Question);
@@ -1060,43 +1069,53 @@ SendConfirmationDialog::SendConfirmationDialog(const QString& title, const QStri
     setInformativeText(informative_text);
     setDetailedText(detailed_text);
     setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel);
+    if (always_show_unsigned || !enable_send) addButton(QMessageBox::Save);
     setDefaultButton(QMessageBox::Cancel);
     yesButton = button(QMessageBox::Yes);
     if (confirmButtonText.isEmpty()) {
         confirmButtonText = yesButton->text();
     }
-    updateYesButton();
+    m_psbt_button = button(QMessageBox::Save);
+    updateButtons();
     connect(&countDownTimer, &QTimer::timeout, this, &SendConfirmationDialog::countDown);
 }
 
 int SendConfirmationDialog::exec()
 {
-    updateYesButton();
+    updateButtons();
     countDownTimer.start(1s);
     return QMessageBox::exec();
 }
 
 void SendConfirmationDialog::countDown()
 {
     secDelay--;
-    updateYesButton();
+    updateButtons();
 
     if(secDelay <= 0)
     {
         countDownTimer.stop();
     }
 }
 
-void SendConfirmationDialog::updateYesButton()
+void SendConfirmationDialog::updateButtons()
 {
     if(secDelay > 0)
     {
         yesButton->setEnabled(false);
-        yesButton->setText(confirmButtonText + " (" + QString::number(secDelay) + ")");
+        yesButton->setText(confirmButtonText + (m_enable_send ? (" (" + QString::number(secDelay) + ")") : QString("")));
+        if (m_psbt_button) {
+            m_psbt_button->setEnabled(false);
+            m_psbt_button->setText(m_psbt_button_text + " (" + QString::number(secDelay) + ")");
+        }
     }
     else
     {
-        yesButton->setEnabled(true);
+        yesButton->setEnabled(m_enable_send);
         yesButton->setText(confirmButtonText);
+        if (m_psbt_button) {
+            m_psbt_button->setEnabled(true);
+            m_psbt_button->setText(m_psbt_button_text);
+        }
     }
 }

src/qt/sendcoinsdialog.h

@@ -112,25 +112,28 @@ private Q_SLOTS:
 };
 
 
-
+#define SEND_CONFIRM_DELAY   3
 class SendConfirmationDialog : public QMessageBox
 {
     Q_OBJECT
 
 public:
     SendConfirmationDialog(const QString &title, const QString &text, int secDelay = 0, QWidget *parent = nullptr);
-    SendConfirmationDialog(const QString& title, const QString& text, const QString& informative_text = "", const QString& detailed_text = "", int secDelay = 0, const QString& confirmText = "", QWidget* parent = nullptr);
+    SendConfirmationDialog(const QString& title, const QString& text, const QString& informative_text = "", const QString& detailed_text = "", int secDelay = SEND_CONFIRM_DELAY, bool enable_send = true, bool always_show_unsigned = true, QWidget* parent = nullptr);
     int exec() override;
 
 private Q_SLOTS:
     void countDown();
-    void updateYesButton();
+    void updateButtons();
 
 private:
     QAbstractButton *yesButton;
+    QAbstractButton *m_psbt_button;
     QTimer countDownTimer;
     int secDelay;
-    QString confirmButtonText;
+    QString confirmButtonText{tr("Send")};
+    bool m_enable_send;
+    QString m_psbt_button_text{tr("Create Unsigned")};
 };
 
 #endif // BITCOIN_QT_SENDCOINSDIALOG_H