Defer transaction signing until user clicks Send

[151henry151]

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.

[DrahtBot]

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 <!--meta-tag:bot-skip--> 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, add musig2 test (partial) 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.

[151henry151]

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]

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

[151henry151]

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

[hebasto]

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]

cc @achow101 @furszy

[achow101]

Concept ACK

[151henry151]

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]

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

[johnny9]

Double check this works with locked encrypted wallets

[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]

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.

[151henry151]

@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

[151henry151]

Thanks for the additional feedback; I've made the changes and I tested locally, I think it is all correct. Please check it out for me and see if you find any other errors or problems with it.

[151henry151]

@johnny9 @achow101 I think this is ready for review, if anybody wants to check it out?

[151henry151]

Just checking in; anything further I can do here to get this ready for merge?

[maflcko]

According to https://btctranscripts.com/bitcoin-core-dev-tech/2026-05/qml-planning and https://btctranscripts.com/bitcoin-core-dev-tech/2026-05/qml-update, there seems to be momentum toward QML, so review in the legacy gui is reduced for now?

[151henry151]

Rebased onto current master (4b91316). Updated for the current fillPSBT/PSBTFillOptions API and post–gui#807 createTransaction flow.

prepareTransaction now always calls createTransaction with sign=false; internal wallets sign via fillPSBT only after the user clicks Send, with UnlockContext at prepare and send (encrypted wallets). The confirmation dialog labels tx size as unsigned. This should also subsume the overlapping walletmodel.cpp hunk in bitcoin/bitcoin#33112.

Tested with cmake --build build --target bitcoin-qt test_bitcoin-qt and ./build/bin/test_bitcoin-qt --run_all — passed.

Re QML: agreed there's momentum toward a QML replacement, but this is a small fix for the GUI we still ship today (#30070); I don't think that direction makes this redundant in the meantime.

[151henry151]

CI on the latest push is mostly failing at "Restore caches" with warpbuilds/cache/restore@v1 not allowed on this repo — could that be from the rebased master CI workflow rather than the patch itself, or did I miss something I should have done?

[hebasto]

CI on the latest push is mostly failing at "Restore caches" with warpbuilds/cache/restore@v1 not allowed on this repo — could that be from the rebased master CI workflow rather than the patch itself, or did I miss something I should have done?

Should work now.