Skip to content

Merge bitcoin-core/gui#555: Restore Send button when using external signer #7271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 1 commit into from
Apr 9, 2026
Merged

Merge bitcoin-core/gui#555: Restore Send button when using external signer #7271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 113 additions & 86 deletions src/qt/sendcoinsdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,80 @@ bool SendCoinsDialog::send(const QList<SendCoinsRecipient>& recipients, QString&
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);
Comment on lines +509 to +513
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use neutral, translated copy in the new PSBT/external-signer messages.

presentPSBT() is also used for the partially signed fallback on Line 637, so "Unsigned Transaction" is misleading there. The new strings here and in the external-signer error boxes are also hard-coded English, so they will not be picked up by Qt translations.

Also applies to: 557-565

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/qt/sendcoinsdialog.cpp` around lines 509 - 513, presentPSBT() shows
hard-coded English and a misleading title ("Unsigned Transaction"); replace the
literals with neutral, translatable strings using QObject::tr() (e.g., title
like tr("PSBT") or tr("Partially Signed Transaction") and informative text like
tr("The PSBT has been copied to the clipboard. You can also save it.")), and
update the other occurrences (the block around the 557-565 region and any
external-signer error boxes) to use tr() as well so translations can pick them
up and the wording is accurate for both unsigned and partially-signed fallbacks.

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);
Comment on lines +536 to +539
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Check the file write before telling the user the PSBT was saved.

If the file cannot be opened or written, this still emits a success message and the dialog is closed afterward, leaving the user with only the clipboard copy. Please validate the stream state and surface an error instead.

Proposed fix 
         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);
+        const std::string psbt = ssTx.str();
+        out.write(psbt.data(), psbt.size());
+        out.close();
+        if (!out) {
+            Q_EMIT message(tr("PSBT save failed"),
+                           tr("Could not write the PSBT to \"%1\".").arg(filename),
+                           CClientUIInterface::MSG_ERROR);
+            return;
+        }
+        Q_EMIT message(tr("PSBT saved"), tr("PSBT saved to disk"), CClientUIInterface::MSG_INFORMATION);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/qt/sendcoinsdialog.cpp` around lines 536 - 539, The code currently writes
ssTx to an ofstream and immediately emits a success message via Q_EMIT
message(...) without checking the stream result; update the sendcoinsdialog.cpp
write block to validate that the file was opened and the write succeeded by
testing out.is_open()/out.good() (or checking out.fail()/out) after writing
ssTx.str(), and if the stream indicates failure emit an error message via Q_EMIT
message(tr("PSBT save error"), /* descriptive message including filename */ ,
CClientUIInterface::MSG_ERROR) instead of the success message; only emit the
existing success Q_EMIT message(tr("PSBT saved"), ...) when the write completed
successfully, and keep closing the stream as needed.

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 @@ -510,7 +584,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 @@ -523,102 +599,53 @@ 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);
Comment on lines +607 to +612
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle fillPSBT() failures explicitly instead of asserting.

fillPSBT() is a runtime wallet call that already returns TransactionError, not an invariant check. In release builds these asserts disappear, so a failed prefill can still fall through to presentPSBT() or signWithExternalSigner() with a half-populated PSBT.

Also applies to: 623-626

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/qt/sendcoinsdialog.cpp` around lines 607 - 612, Replace the assertions
after calls to model->wallet().fillPSBT(...) with explicit error handling: check
the returned TransactionError (err) and the completion flag (complete) for both
fillPSBT invocations, show an appropriate user-facing error/dialog message when
err != TransactionError::OK or when complete is unexpectedly true/false
(depending on the call site), and return early instead of proceeding to
presentPSBT(...) or signWithExternalSigner(...) so a partially-filled PSBT is
never used; reference the model->wallet().fillPSBT call, the TransactionError
enum, the psbtx/complete variables, and the downstream presentPSBT and
signWithExternalSigner calls to locate where to add the checks and early
returns.

} 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
model->sendCoins(*m_current_transaction, m_coin_control->IsUsingCoinJoin());
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;
Q_EMIT coinsSent(m_current_transaction->getWtx()->GetHash());
}
case QMessageBox::Discard:
break;
default:
assert(false);
} // msgBox.exec()
} 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());
}
if (!send_failure) {
accept();
Expand Down
13 changes: 13 additions & 0 deletions src/qt/sendcoinsdialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,24 @@ public Q_SLOTS:
bool fFeeMinimized{true};
bool fKeepChangeAddress;

// 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 @@ -119,6 +130,8 @@ class SendConfirmationDialog : public QMessageBox
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 = 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
Loading