Skip to content

Commit

Permalink
Merge #555: Restore Send button when using external signer
Browse files Browse the repository at this point in the history 
2efdfb8 gui: restore Send for external signer (Sjors Provoost)
4b5a6cd refactor: helper function signWithExternalSigner() (Sjors Provoost)
026b5b4 move-only: helper function to present PSBT (Sjors Provoost)

Pull request description:

  Fixes #551

  For the simplest use case of a wallet with one external signer and "PSBT Controls" disabled in settings (the default), the send dialog will behave the same as when using a wallet with private keys. I.e. there's no "Create Unsigned" button.

  When PSBT controls are turned on, you can now actually make a PSBT with signing it; before this PR that button would trigger a sign event and would broadcast the transaction.

  In case a multisig, the Send button will sign on the device, and then fall back to presenting a PSBT (same behavior as before #441).

  This PR starts with two refactoring commits to move some stuff into a helper function for improved readability in general, and to make the main commit easier to review.

ACKs for top commit:
  jonatack:
    utACK 2efdfb8 diff review since my last review, code re-review, rebased to current master, verified clean debug build of each commit
  luke-jr:
    utACK 2efdfb8

Tree-SHA512: e8731a0ef9e87564b2676c7b022b742d9621bba964c19dba9fd9f6961eb608737a9e1a22c0a3c8b2f2f6d583bba067606ee8392422e82082deefb20ea7b88c7c
  • Loading branch information
@hebasto
hebasto committed Mar 17, 2022
2 parents 74f8c55 + 2efdfb8 commit aece566
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 93 deletions.
205 changes: 112 additions & 93 deletions src/qt/sendcoinsdialog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,80 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
return true;
}

void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
{
// Serialize the PSBT
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << psbtx;
GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
QMessageBox msgBox;
msgBox.setText("Unsigned Transaction");
msgBox.setInformativeText("The PSBT has been copied to the clipboard. You can also save it.");
msgBox.setStandardButtons(QMessageBox::Save | QMessageBox::Discard);
msgBox.setDefaultButton(QMessageBox::Discard);
switch (msgBox.exec()) {
case QMessageBox::Save: {
QString selectedFilter;
QString fileNameSuggestion = "";
bool first = true;
for (const SendCoinsRecipient &rcp : m_current_transaction->getRecipients()) {
if (!first) {
fileNameSuggestion.append(" - ");
}
QString labelOrAddress = rcp.label.isEmpty() ? rcp.address : rcp.label;
QString amount = BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount);
fileNameSuggestion.append(labelOrAddress + "-" + amount);
first = false;
}
fileNameSuggestion.append(".psbt");
QString filename = GUIUtil::getSaveFileName(this,
tr("Save Transaction Data"), fileNameSuggestion,
//: Expanded name of the binary PSBT file format. See: BIP 174.
tr("Partially Signed Transaction (Binary)") + QLatin1String(" (*.psbt)"), &selectedFilter);
if (filename.isEmpty()) {
return;
}
std::ofstream out{filename.toLocal8Bit().data(), std::ofstream::out | std::ofstream::binary};
out << ssTx.str();
out.close();
Q_EMIT message(tr("PSBT saved"), "PSBT saved to disk", CClientUIInterface::MSG_INFORMATION);
break;
}
case QMessageBox::Discard:
break;
default:
assert(false);
} // msgBox.exec()
}

bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) {
TransactionError err;
try {
err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
} catch (const std::runtime_error& e) {
QMessageBox::critical(nullptr, tr("Sign failed"), e.what());
return false;
}
if (err == TransactionError::EXTERNAL_SIGNER_NOT_FOUND) {
//: "External signer" means using devices such as hardware wallets.
QMessageBox::critical(nullptr, tr("External signer not found"), "External signer not found");
return false;
}
if (err == TransactionError::EXTERNAL_SIGNER_FAILED) {
//: "External signer" means using devices such as hardware wallets.
QMessageBox::critical(nullptr, tr("External signer failure"), "External signer failure");
return false;
}
if (err != TransactionError::OK) {
tfm::format(std::cerr, "Failed to sign PSBT");
processSendCoinsReturn(WalletModel::TransactionCreationFailed);
return false;
}
// fillPSBT does not always properly finalize
complete = FinalizeAndExtractPSBT(psbtx, mtx);
return true;
}

void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
{
if(!model || !model->getOptionsModel())
Expand All @@ -411,7 +485,9 @@ 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);
const bool enable_send{!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner()};
const bool always_show_unsigned{model->getOptionsModel()->getEnablePSBTControls()};
auto confirmationDialog = new SendConfirmationDialog(confirmation, question_string, informative_text, detailed_text, SEND_CONFIRM_DELAY, enable_send, always_show_unsigned, 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 @@ -424,49 +500,50 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)

bool send_failure = false;
if (retval == QMessageBox::Save) {
// "Create Unsigned" clicked
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
// Always fill without signing first. This prevents an external signer
// from being called prematurely and is not expensive.
TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, nullptr, psbtx, complete);
// Fill without signing
TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
assert(!complete);
assert(err == TransactionError::OK);

// 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()) {
try {
err = model->wallet().fillPSBT(SIGHASH_ALL, true /* sign */, true /* bip32derivs */, nullptr, psbtx, complete);
} catch (const std::runtime_error& e) {
QMessageBox::critical(nullptr, tr("Sign failed"), e.what());
send_failure = true;
return;
}
if (err == TransactionError::EXTERNAL_SIGNER_NOT_FOUND) {
//: "External signer" means using devices such as hardware wallets.
QMessageBox::critical(nullptr, tr("External signer not found"), "External signer not found");
send_failure = true;
return;
}
if (err == TransactionError::EXTERNAL_SIGNER_FAILED) {
//: "External signer" means using devices such as hardware wallets.
QMessageBox::critical(nullptr, tr("External signer failure"), "External signer failure");
send_failure = true;
return;
}
if (err != TransactionError::OK) {
tfm::format(std::cerr, "Failed to sign PSBT");
processSendCoinsReturn(WalletModel::TransactionCreationFailed);
send_failure = true;
return;
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
// Always fill without signing first. This prevents an external signer
// from being called prematurely and is not expensive.
TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
assert(!complete);
assert(err == TransactionError::OK);
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);
}
}
// fillPSBT does not always properly finalize
complete = FinalizeAndExtractPSBT(psbtx, mtx);
}

// 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 @@ -476,64 +553,6 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
} else {
send_failure = true;
}
return;
}

// Copy PSBT to clipboard and offer to save
assert(!complete);
// Serialize the PSBT
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << psbtx;
GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
QMessageBox msgBox;
msgBox.setText("Unsigned Transaction");
msgBox.setInformativeText("The PSBT has been copied to the clipboard. You can also save it.");
msgBox.setStandardButtons(QMessageBox::Save | QMessageBox::Discard);
msgBox.setDefaultButton(QMessageBox::Discard);
switch (msgBox.exec()) {
case QMessageBox::Save: {
QString selectedFilter;
QString fileNameSuggestion = "";
bool first = true;
for (const SendCoinsRecipient &rcp : m_current_transaction->getRecipients()) {
if (!first) {
fileNameSuggestion.append(" - ");
}
QString labelOrAddress = rcp.label.isEmpty() ? rcp.address : rcp.label;
QString amount = BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount);
fileNameSuggestion.append(labelOrAddress + "-" + amount);
first = false;
}
fileNameSuggestion.append(".psbt");
QString filename = GUIUtil::getSaveFileName(this,
tr("Save Transaction Data"), fileNameSuggestion,
//: Expanded name of the binary PSBT file format. See: BIP 174.
tr("Partially Signed Transaction (Binary)") + QLatin1String(" (*.psbt)"), &selectedFilter);
if (filename.isEmpty()) {
return;
}
std::ofstream out{filename.toLocal8Bit().data(), std::ofstream::out | std::ofstream::binary};
out << ssTx.str();
out.close();
Q_EMIT message(tr("PSBT saved"), "PSBT saved to disk", CClientUIInterface::MSG_INFORMATION);
break;
}
case QMessageBox::Discard:
break;
default:
assert(false);
} // msgBox.exec()
} 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
13 changes: 13 additions & 0 deletions src/qt/sendcoinsdialog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,24 @@ public Q_SLOTS:
bool fFeeMinimized;
const PlatformStyle *platformStyle;

// Copy PSBT to clipboard and offer to save it.
void presentPSBT(PartiallySignedTransaction& psbt);
// Process WalletModel::SendCoinsReturn and generate a pair consisting
// of a message and message flags for use in Q_EMIT message().
// Additional parameter msgArg can be used via .arg(msgArg).
void processSendCoinsReturn(const WalletModel::SendCoinsReturn &sendCoinsReturn, const QString &msgArg = QString());
void minimizeFeeSection(bool fMinimize);
// Format confirmation message
bool PrepareSendText(QString& question_string, QString& informative_text, QString& detailed_text);
/* Sign PSBT using external signer.
*
* @param[in,out] psbtx the PSBT to sign
* @param[in,out] mtx needed to attempt to finalize
* @param[in,out] complete whether the PSBT is complete (a successfully signed multisig transaction may not be complete)
*
* @returns false if any failure occurred, which may include the user rejection of a transaction on the device.
*/
bool signWithExternalSigner(PartiallySignedTransaction& psbt, CMutableTransaction& mtx, bool& complete);
void updateFeeMinimizedLabel();
void updateCoinControlState();

Expand Down Expand Up @@ -117,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 aece566

Please sign in to comment.