From 4ed2a3bd0ce16efefa0b689fb1457a5273cf528f Mon Sep 17 00:00:00 2001
From: Sjors Provoost <sjors@sprovoost.nl>
Date: Mon, 21 Feb 2022 13:32:21 +0100
Subject: [PATCH] gui: restore Send for external signer

Before this change the send confirmation dialog would keep the Send option disabled. The Create Unsigned choice would actually send. This is potentially confusing.

With this change the Create Unsigned button will not attempt to sign and always produce a PSBT. The Send button will attempt to sign, and only return a PSBT if more signatures are needed.

When using an external signer, the Create Unsigned option only appears when PSBT controls are enabled in the wallet settings.

Closes #551
---
 src/qt/sendcoinsdialog.cpp | 52 ++++++++++++++++++++------------------
 src/qt/sendcoinsdialog.h   |  2 ++
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 0b041c54166..d5457ad35a7 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -490,7 +490,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
     assert(m_current_transaction);
 
     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);
+    auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, /*enable_send=*/!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner(), /*always_show_unsigned=*/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());
@@ -502,19 +502,38 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
     }
 
     bool send_failure = false;
-    if (retval == QMessageBox::Save) {
+    if (retval == QMessageBox::Save) { // Create Unsigned clicked
         CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
         PartiallySignedTransaction psbtx(mtx);
-        bool complete = false;
+        // Copy PSBT to clipboard and offer to save
+        presentPSBT(psbtx);
+    } else { // Send clicked
+        assert(!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner());
+        bool broadcast = true;
         if (model->wallet().hasExternalSigner()) {
+            CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
+            PartiallySignedTransaction psbtx(mtx);
+            bool complete = false;
             send_failure = !signWithExternalSigner(psbtx, mtx, complete);
+            // Don't broadcast when user rejects it on the device or there's a failure:
+            broadcast = complete && !send_failure;
+            if (!send_failure) {
+                // A transaction signed with an external signer is not always complete,
+                // e.g. in a multisig wallet.
+                if (complete) {
+                    // Prepare transaction for broadcast transaction if complete
+                    const CTransactionRef tx = MakeTransactionRef(mtx);
+                    m_current_transaction->setWtx(tx);
+                } else {
+                    presentPSBT(psbtx);
+                }
+            }
         }
 
-        // Broadcast transaction if complete (even with an external signer this
-        // is not always the case, e.g. in a multisig wallet).
-        if (complete) {
-            const CTransactionRef tx = MakeTransactionRef(mtx);
-            m_current_transaction->setWtx(tx);
+        // Broadcast the transaction, unless an external was used and it
+        // failed, or more signatures are needed.
+        if (broadcast) {
+            // now send the prepared transaction
             WalletModel::SendCoinsReturn sendStatus = model->sendCoins(*m_current_transaction);
             // process sendStatus and on error generate message shown to user
             processSendCoinsReturn(sendStatus);
@@ -524,23 +543,6 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
             } else {
                 send_failure = true;
             }
-            return;
-        }
-
-        // Copy PSBT to clipboard and offer to save
-        assert(!complete);
-        presentPSBT(psbtx);
-    } else {
-        assert(!model->wallet().privateKeysDisabled());
-        // now send the prepared transaction
-        WalletModel::SendCoinsReturn sendStatus = model->sendCoins(*m_current_transaction);
-        // process sendStatus and on error generate message shown to user
-        processSendCoinsReturn(sendStatus);
-
-        if (sendStatus.status == WalletModel::OK) {
-            Q_EMIT coinsSent(m_current_transaction->getWtx()->GetHash());
-        } else {
-            send_failure = true;
         }
     }
     if (!send_failure) {
diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h
index bd856ced1e1..87b2b560bdb 100644
--- a/src/qt/sendcoinsdialog.h
+++ b/src/qt/sendcoinsdialog.h
@@ -128,6 +128,8 @@ class SendConfirmationDialog : public QMessageBox
 
 public:
     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);
+    /* Returns QMessageBox::Cancel, QMessageBox::Yes when send is
+       clicked and QMessageBox::Save when Create Unsigned is clicked. */
     int exec() override;
 
 private Q_SLOTS: