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.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
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.
Reviewers, this pull request conflicts with the following ones:
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.
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.
146@@ -147,7 +147,7 @@ bool WalletModel::validateAddress(const QString& address) const
147 return IsValidDestinationString(address.toStdString());
148 }
149
150-WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl)
151+WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransaction &transaction, const CCoinControl& coinControl, bool sign)
prepareTransaction has only a single caller that already sets sign = false. I don’t think this needs to take a parameter, it can just always not sign.
546+ // Sign the transaction now that the user has confirmed they want to send.
547+ CMutableTransaction mtx = CMutableTransaction{*(m_current_transaction->getWtx())};
548+ PartiallySignedTransaction psbtx(mtx);
549+ bool complete = false;
550+ // Fill and sign the PSBT
551+ const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/false, /*n_signed=*/nullptr, psbtx, complete)};
fillPSBT on it is being done for all of the other cases in this function, and hence is now duplicated 3 times. This can be refactored so that the PSBT is always created and filled at the top of the function, and each action can do whatever specific thing it needs to do with that PSBT.
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
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.
Need to move the UnlockContext when you sign the transaction otherwise it fails to send now.
https://github.com/user-attachments/assets/6b7e6894-b803-4753-9f20-66541af63633
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.
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.