Defer transaction signing until user clicks Send by 151henry151 · Pull Request #915 · bitcoin-core/gui

151henry151 · 2025-11-22T19:19:22Z

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.

DrahtBot · 2025-11-22T19:19:37Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	achow101

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#bitcoin/bitcoin/33112 (wallet: relax external_signer flag constraints by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

tr("%1 kvB (unsigned)", "PSBT transaction creation").arg((double)m_current_transaction->getTransactionSize() / 1000, 0, 'g', 3) in src/qt/sendcoinsdialog.cpp

^{2026-03-16 01:01:50}

151henry151 · 2025-11-23T18:02:32Z

It looks like the one failing check is a CI issue with disk space too low. I'm not sure how to address that. I think the code of the PR is correct.

151henry151 · 2025-11-29T21:51:40Z

@achow101 would you like to check this out and let me know if it looks OK?

151henry151 · 2026-01-18T18:40:14Z

Should I push an empty commit to trigger a re-run of the CI here, or is there some other action needed?

hebasto · 2026-01-19T15:59:32Z

Should I push an empty commit to trigger a re-run of the CI here, or is there some other action needed?

No action needed.

hebasto · 2026-01-19T15:59:57Z

cc @achow101 @furszy

achow101

Concept ACK

src/qt/walletmodel.cpp

src/qt/sendcoinsdialog.cpp

151henry151 · 2026-02-11T04:56:32Z

For the rebase, I kept master's new createTransaction usage and error flow, wired in the defer-signing logic (sign parameter), and re-added the AmountExceedsBalance check after a successful result. I initially missed updating that to AmountExceedsBalance, but I’ve corrected it. If anyone wants to check this out and let me know if it looks correct, I'd appreciate any feedback. Thanks

151henry151 · 2026-03-01T21:44:51Z

Would anyone like to take another look at this? Any review or feedback welcomed

johnny9

Double check this works with locked encrypted wallets

src/qt/sendcoinsdialog.cpp

johnny9

Need to move the UnlockContext when you sign the transaction otherwise it fails to send now.

Screen.Recording.2026-03-04.at.11.32.40.AM.mov

151henry151 · 2026-03-04T16:42:52Z

Thanks @johnny9 , I'll take a closer look this evening

johnny9

Moving the UnlockContext in the "Send" click flow resolved the previous issue. I think the previous UnlockContext is no longer needed.

I also see that the send confirmation dialog isn't as accurate as it was previously so should adjust what the dialog says or expose the correct fee estimated size through the model.

src/qt/sendcoinsdialog.cpp

johnny9 · 2026-03-08T15:09:39Z

src/qt/sendcoinsdialog.cpp

This is a little different now that the current transaction isn't signed yet so the kvB is different now than before. Can either change the dialog to make this clear or adjust this size to be the same as the estimated size the fee calculation uses.

For this example vsize should equal the fee (208 kvB and 208 sat fee). Without your changes they were same and now the size of the transaction doesnt include the signed portion so it isn't matching the fee.

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.

151henry151 · 2026-03-16T01:03:40Z

@johnny9 I think I've fixed it correctly; I tested locally (built the gui and tested manually) and it seems like the issues are resolved. Could you test it again to confirm, and take a look at the changes to see if there are any mistakes or problems you can see?

johnny9

I actually think I was wrong and we need both UnlockContexts. Sorry to flp-flop on that. Also take a look at achow's comment again. Seems like you didnt remove the parameter he mentioned. Other than those things I think this looks good now

johnny9 · 2026-03-18T03:02:27Z

src/qt/sendcoinsdialog.cpp

-    WalletModel::UnlockContext ctx(model->requestUnlock());
-    if(!ctx.isValid())
-    {
-        // Unlock wallet was cancelled


I think I was wrong about removing this. I think you will need unlock here too to be sure you can get a change address.

src/qt/walletmodel.cpp

johnny9 · 2026-03-18T03:05:32Z

src/qt/walletmodel.h

@@ -95,7 +95,7 @@ class WalletModel : public QObject
    };

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


sign parameter can be removed

hebasto changed the title ~~qt: Defer transaction signing until user clicks Send~~ Defer transaction signing until user clicks Send Nov 22, 2025

DrahtBot added the CI failed label Nov 22, 2025

DrahtBot mentioned this pull request Nov 23, 2025

wallet: relax external_signer flag constraints bitcoin/bitcoin#33112

Open

maflcko closed this Dec 17, 2025

maflcko reopened this Dec 17, 2025

hebasto added Wallet and removed CI failed labels Jan 19, 2026

achow101 reviewed Jan 19, 2026

View reviewed changes

src/qt/walletmodel.cpp Show resolved Hide resolved

src/qt/sendcoinsdialog.cpp Show resolved Hide resolved

151henry151 force-pushed the fix-30070-defer-signing branch from 58adbcf to 3c61ba5 Compare January 20, 2026 17:25

DrahtBot added the CI failed label Jan 20, 2026

DrahtBot mentioned this pull request Feb 7, 2026

refactor: interfaces, make 'createTransaction' less error-prone #807

Merged

DrahtBot added the Needs rebase label Feb 10, 2026

151henry151 force-pushed the fix-30070-defer-signing branch from 3c61ba5 to fa548a3 Compare February 11, 2026 01:52

DrahtBot removed the Needs rebase label Feb 11, 2026

151henry151 force-pushed the fix-30070-defer-signing branch from fa548a3 to 9b65413 Compare February 11, 2026 04:49

DrahtBot removed the CI failed label Feb 11, 2026

johnny9 reviewed Mar 4, 2026

View reviewed changes

src/qt/sendcoinsdialog.cpp Outdated Show resolved Hide resolved

johnny9 suggested changes Mar 4, 2026

View reviewed changes

151henry151 force-pushed the fix-30070-defer-signing branch from 9b65413 to 39a5fed Compare March 5, 2026 22:05

DrahtBot added the CI failed label Mar 6, 2026

johnny9 reviewed Mar 8, 2026

View reviewed changes

151henry151 force-pushed the fix-30070-defer-signing branch from 39a5fed to 6a999c2 Compare March 16, 2026 01:01

DrahtBot removed the CI failed label Mar 16, 2026

151henry151 requested a review from johnny9 March 16, 2026 15:59

johnny9 reviewed Mar 18, 2026

View reviewed changes

Conversation

151henry151 commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

151henry151 commented Nov 23, 2025

Uh oh!

151henry151 commented Nov 29, 2025

Uh oh!

151henry151 commented Jan 18, 2026

Uh oh!

hebasto commented Jan 19, 2026

Uh oh!

hebasto commented Jan 19, 2026

Uh oh!

achow101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

151henry151 commented Feb 11, 2026

Uh oh!

151henry151 commented Mar 1, 2026

Uh oh!

johnny9 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnny9 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

151henry151 commented Mar 4, 2026

Uh oh!

johnny9 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnny9 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

johnny9 Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

151henry151 commented Mar 16, 2026

Uh oh!

johnny9 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnny9 Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnny9 Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

151henry151 commented Nov 22, 2025 •

edited

Loading

DrahtBot commented Nov 22, 2025 •

edited

Loading

johnny9 left a comment •

edited

Loading

johnny9 left a comment •

edited

Loading

johnny9 Mar 8, 2026 •

edited

Loading

johnny9 left a comment •

edited

Loading