Skip to content

Commit 39a5fed

Browse files
committed
qt: Defer transaction signing until user clicks Send
This fixes issue #30070 where creating unsigned PSBTs from the GUI would fail because the transaction was already signed during preparation, causing legacy inputs to have non-empty scriptSig fields. The fix defers signing until the user explicitly clicks 'Send', allowing truly unsigned PSBTs to be created while still supporting fee calculation.
1 parent 91a8e9b commit 39a5fed

3 files changed

Lines changed: 36 additions & 4 deletions

File tree

src/qt/sendcoinsdialog.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,16 @@ 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

287289
updateCoinControlState();
288290

289291
CCoinControl coin_control = *m_coin_control;
290292
coin_control.m_allow_other_inputs = !coin_control.HasSelected(); // future, could introduce a checkbox to customize this value.
291-
prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control);
293+
prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control, /*sign=*/false);
292294

293295
// process prepareStatus and on error generate message shown to user
294296
processSendCoinsReturn(prepareStatus,
@@ -515,6 +517,12 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
515517
presentPSBT(psbtx);
516518
} else {
517519
// "Send" clicked
520+
WalletModel::UnlockContext ctx(model->requestUnlock());
521+
if (!ctx.isValid()) {
522+
fNewRecipientAllowed = true;
523+
return;
524+
}
525+
518526
assert(!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner());
519527
bool broadcast = true;
520528
if (model->wallet().hasExternalSigner()) {
@@ -540,6 +548,24 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
540548
presentPSBT(psbtx);
541549
}
542550
}
551+
} else {
552+
// Sign the transaction now that the user has confirmed they want to send.
553+
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
554+
PartiallySignedTransaction psbtx(mtx);
555+
bool complete = false;
556+
// Fill and sign the PSBT
557+
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/false, /*n_signed=*/nullptr, psbtx, complete)};
558+
if (err || !complete) {
559+
Q_EMIT message(tr("Send Coins"), tr("Failed to sign transaction."),
560+
CClientUIInterface::MSG_ERROR);
561+
send_failure = true;
562+
broadcast = false;
563+
} else {
564+
// Extract the signed transaction
565+
CHECK_NONFATAL(FinalizeAndExtractPSBT(psbtx, mtx));
566+
const CTransactionRef tx = MakeTransactionRef(mtx);
567+
m_current_transaction->setWtx(tx);
568+
}
543569
}
544570

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

src/qt/walletmodel.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ bool WalletModel::validateAddress(const QString& address) const
148148
return IsValidDestinationString(address.toStdString());
149149
}
150150

151-
WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl)
151+
WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl, bool sign)
152152
{
153153
transaction.getWtx() = nullptr; // reset tx output
154154

@@ -203,7 +203,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
203203

204204
try {
205205
auto& newTx = transaction.getWtx();
206-
const auto& res = m_wallet->createTransaction(vecSend, coinControl, /*sign=*/!wallet().privateKeysDisabled(), /*change_pos=*/std::nullopt);
206+
// Only sign if explicitly requested via the sign parameter (e.g. when user clicks Send).
207+
const bool should_sign = sign && !wallet().privateKeysDisabled();
208+
const auto& res = m_wallet->createTransaction(vecSend, coinControl, should_sign, /*change_pos=*/std::nullopt);
207209
if (!res) {
208210
Q_EMIT message(tr("Send Coins"), QString::fromStdString(util::ErrorString(res).translated),
209211
CClientUIInterface::MSG_ERROR);
@@ -217,6 +219,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
217219
transaction.reassignAmounts(static_cast<int>(res->change_pos.value_or(-1)));
218220
}
219221

222+
if (!fSubtractFeeFromAmount && (total + nFeeRequired) > nBalance) {
223+
return SendCoinsReturn(AmountExceedsBalance);
224+
}
225+
220226
// Reject absurdly high fee. (This can never happen because the
221227
// wallet never creates transactions with fee greater than
222228
// m_default_max_tx_fee. This merely a belt-and-suspenders check).

src/qt/walletmodel.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class WalletModel : public QObject
9595
};
9696

9797
// prepare transaction for getting txfee before sending coins
98-
SendCoinsReturn prepareTransaction(WalletModelTransaction &transaction, const wallet::CCoinControl& coinControl);
98+
SendCoinsReturn prepareTransaction(WalletModelTransaction &transaction, const wallet::CCoinControl& coinControl, bool sign = false);
9999

100100
// Send coins to a list of recipients
101101
void sendCoins(WalletModelTransaction& transaction);

0 commit comments

Comments
 (0)