Skip to content

Commit 3c61ba5

Browse files
committed
qt: Defer transaction signing until user clicks Send
Fixes #30070 When creating an unsigned PSBT from the GUI, the transaction was already signed during preparation, causing legacy inputs to have non-empty scriptSig fields. The PSBT parser then rejects them. This defers signing until the user clicks "Send" instead of signing during preparation. Fee calculation still works since transactions can be created without signing. Follows the approach suggested by @achow101 in the issue comments.
1 parent 0690514 commit 3c61ba5

2 files changed

Lines changed: 25 additions & 5 deletions

File tree

src/qt/sendcoinsdialog.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,8 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
281281
}
282282

283283
// prepare transaction for getting txFee earlier
284+
// Create unsigned transaction to support creating unsigned PSBTs.
285+
// Signing is deferred until the user clicks "Send".
284286
m_current_transaction = std::make_unique<WalletModelTransaction>(recipients);
285287
WalletModel::SendCoinsReturn prepareStatus;
286288

@@ -500,11 +502,13 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
500502
return;
501503
}
502504

505+
// Create PSBT from the prepared transaction
506+
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
507+
PartiallySignedTransaction psbtx(mtx);
508+
503509
bool send_failure = false;
504510
if (retval == QMessageBox::Save) {
505511
// "Create Unsigned" clicked
506-
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
507-
PartiallySignedTransaction psbtx(mtx);
508512
bool complete = false;
509513
// Fill without signing
510514
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
@@ -518,8 +522,6 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
518522
assert(!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner());
519523
bool broadcast = true;
520524
if (model->wallet().hasExternalSigner()) {
521-
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
522-
PartiallySignedTransaction psbtx(mtx);
523525
bool complete = false;
524526
// Always fill without signing first. This prevents an external signer
525527
// from being called prematurely and is not expensive.
@@ -540,6 +542,22 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
540542
presentPSBT(psbtx);
541543
}
542544
}
545+
} else {
546+
// Sign the transaction now that the user has confirmed they want to send.
547+
bool complete = false;
548+
// Fill and sign the PSBT
549+
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/false, /*n_signed=*/nullptr, psbtx, complete)};
550+
if (err || !complete) {
551+
Q_EMIT message(tr("Send Coins"), tr("Failed to sign transaction."),
552+
CClientUIInterface::MSG_ERROR);
553+
send_failure = true;
554+
broadcast = false;
555+
} else {
556+
// Extract the signed transaction
557+
CHECK_NONFATAL(FinalizeAndExtractPSBT(psbtx, mtx));
558+
const CTransactionRef tx = MakeTransactionRef(mtx);
559+
m_current_transaction->setWtx(tx);
560+
}
543561
}
544562

545563
// Broadcast the transaction, unless an external signer was used and it

src/qt/walletmodel.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
203203
int nChangePosRet = -1;
204204

205205
auto& newTx = transaction.getWtx();
206-
const auto& res = m_wallet->createTransaction(vecSend, coinControl, /*sign=*/!wallet().privateKeysDisabled(), nChangePosRet, nFeeRequired);
206+
// Transaction is not signed during preparation.
207+
const bool should_sign = false;
208+
const auto& res = m_wallet->createTransaction(vecSend, coinControl, should_sign, nChangePosRet, nFeeRequired);
207209
newTx = res ? *res : nullptr;
208210
transaction.setTransactionFee(nFeeRequired);
209211
if (fSubtractFeeFromAmount && newTx)

0 commit comments

Comments
 (0)