Skip to content

Commit

Permalink
gui: restore Send for external signer
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Sjors committed Mar 10, 2022
1 parent e1c1c61 commit 86e3e2c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 25 deletions.
52 changes: 27 additions & 25 deletions src/qt/sendcoinsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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 signer 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);
Expand All @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/qt/sendcoinsdialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 86e3e2c

Please sign in to comment.