Skip to content

Commit 6a999c2

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 6a999c2

3 files changed

Lines changed: 37 additions & 12 deletions

File tree

src/qt/sendcoinsdialog.cpp

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -272,23 +272,18 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
272272
}
273273

274274
fNewRecipientAllowed = false;
275-
WalletModel::UnlockContext ctx(model->requestUnlock());
276-
if(!ctx.isValid())
277-
{
278-
// Unlock wallet was cancelled
279-
fNewRecipientAllowed = true;
280-
return false;
281-
}
282275

283276
// prepare transaction for getting txFee earlier
277+
// Create unsigned transaction to support creating unsigned PSBTs.
278+
// Signing is deferred until the user clicks "Send".
284279
m_current_transaction = std::make_unique<WalletModelTransaction>(recipients);
285280
WalletModel::SendCoinsReturn prepareStatus;
286281

287282
updateCoinControlState();
288283

289284
CCoinControl coin_control = *m_coin_control;
290285
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);
286+
prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control, /*sign=*/false);
292287

293288
// process prepareStatus and on error generate message shown to user
294289
processSendCoinsReturn(prepareStatus,
@@ -357,7 +352,7 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa
357352

358353
// append transaction size
359354
//: When reviewing a newly created PSBT (via Send flow), the transaction fee is shown, with "virtual size" of the transaction displayed for context
360-
question_string.append(" (" + tr("%1 kvB", "PSBT transaction creation").arg((double)m_current_transaction->getTransactionSize() / 1000, 0, 'g', 3) + "): ");
355+
question_string.append(" (" + tr("%1 kvB (unsigned)", "PSBT transaction creation").arg((double)m_current_transaction->getTransactionSize() / 1000, 0, 'g', 3) + "): ");
361356

362357
// append transaction fee value
363358
question_string.append("<span style='color:#aa0000; font-weight:bold;'>");
@@ -515,6 +510,12 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
515510
presentPSBT(psbtx);
516511
} else {
517512
// "Send" clicked
513+
WalletModel::UnlockContext ctx(model->requestUnlock());
514+
if (!ctx.isValid()) {
515+
fNewRecipientAllowed = true;
516+
return;
517+
}
518+
518519
assert(!model->wallet().privateKeysDisabled() || model->wallet().hasExternalSigner());
519520
bool broadcast = true;
520521
if (model->wallet().hasExternalSigner()) {
@@ -540,6 +541,24 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
540541
presentPSBT(psbtx);
541542
}
542543
}
544+
} else {
545+
// Sign the transaction now that the user has confirmed they want to send.
546+
CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
547+
PartiallySignedTransaction psbtx(mtx);
548+
bool complete = false;
549+
// Fill and sign the PSBT
550+
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/false, /*n_signed=*/nullptr, psbtx, complete)};
551+
if (err || !complete) {
552+
Q_EMIT message(tr("Send Coins"), tr("Failed to sign transaction."),
553+
CClientUIInterface::MSG_ERROR);
554+
send_failure = true;
555+
broadcast = false;
556+
} else {
557+
// Extract the signed transaction
558+
CHECK_NONFATAL(FinalizeAndExtractPSBT(psbtx, mtx));
559+
const CTransactionRef tx = MakeTransactionRef(mtx);
560+
m_current_transaction->setWtx(tx);
561+
}
543562
}
544563

545564
// 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)