Improve user dialog when signing multisig psbts

mjdietzx commented at 9:21 pm on July 30, 2024: contributor

Prior to this commit, when a psbt requiring multiple signers is signed via gui the dialogue is vague/confusing. Whether the signer successfully partially signs the psbt or not the same warning is displayed “Could not sign any more inputs.” After signing the tx, the only way for the user to know if their action had any effect is to inspect the psbt (unless it was the last signature that completed the psbt, in which case there is a success dialoge).

Now we indicate when a psbt has been partially signed, and show how many partial signatures the psbt has in the transaction description.

Here is what a “daisy chain” signing flow for a 4-of-4 multisig looks like via gui. Same as in bitcoin core’s functional test wallet_multisig_descriptor_psbt

First signer:

Second signer:

Third signer:

Fourth and final signer (psbt is complete and can be broadcasted):

Before this commit, the first-third signers would just see this after signing. It is at best confusing because it is the same warning a signer would see if they were not in the multisig and no signature was added to the psbt:

DrahtBot commented at 9:21 pm on July 30, 2024: contributor

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	pablomartin4btc, BrandonOdiwuor

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label CI failed on Jul 31, 2024

DrahtBot commented at 0:46 am on July 31, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin-core/gui/runs/28127968944

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

gui: improve user dialog when signing multisig psbts

Prior to this commit, when a psbt requiring multiple signers
is signed via gui the dialog is vague/confusing. Whether the
signer successfully partially signs the psbt or not the same warning
is displayed "Could not sign any more inputs." After signing the tx,
the only way for the user to know if their action had any effect is
to inspect the psbt (unless it was the last signature that completed
the psbt, in which case there is a success dialog).

Now we indicate when a psbt has been partially signed, and show how many
partial signatures the psbt has in the transaction description.

8aec372053

mjdietzx force-pushed on Jul 31, 2024

DrahtBot removed the label CI failed on Jul 31, 2024

hebasto renamed this:
~~gui: improve user dialog when signing multisig psbts~~
Improve user dialog when signing multisig psbts
on Jul 31, 2024

hebasto commented at 7:32 am on July 31, 2024: member

cc @achow101 @furszy

in src/qt/psbtoperationsdialog.cpp:246 in 8aec372053

241     if (num_unsigned > 0) {
242         tx_description.append("<br><br>");
243         tx_description.append(tr("Transaction has %1 unsigned inputs.").arg(QString::number(num_unsigned)));
244+        if (all_inputs_n_partial_sigs_equal && num_partial_sigs > 0) {
245+            tx_description.append(" ");
246+            tx_description.append(tr("Each input has %1 partial signatures.").arg(QString::number(num_partial_sigs)));

pablomartin4btc commented at 4:10 pm on July 31, 2024:

optional nit (IMO makes it more clear that includes the just added partial signature):

0            tx_description.append(tr("Each input now has %1 partial signatures.").arg(QString::number(num_partial_sigs)));

mjdietzx commented at 4:49 pm on July 31, 2024:

Before you sign, if there are already partial sigs on a psbt, this text will show that. For example, 3rd signer in a multisig:

before signing tx_description tells them there are already 2 partial sigs
they sign and see they have successfully added a partial signature. tx_description now tells them there are 3 partial sigs
if they were to try to sign again, then they’d see the warning “Could not sign any more inputs.” at the top of dialog, but tx_description would still show there are 3 psbts

Therefore I think now should not be added

pablomartin4btc commented at 4:55 pm on July 31, 2024:

Ok, thanks.

pablomartin4btc commented at 4:32 pm on July 31, 2024: contributor

Concept ACK

pablomartin4btc commented at 4:48 pm on July 31, 2024: contributor

It would be interesting to see a PSBT window’s output sample when the user signed an input but couldn’t sign another input which is also part of the transaction? (Is that possible?)

mjdietzx commented at 5:05 pm on July 31, 2024: contributor

It would be interesting to see a PSBT window’s output sample when the user signed an input but couldn’t sign another input which is also part of the transaction? (Is that possible?)

It’s possible but I considered it outside the scope of this PR - in such cases behavior is not changed by this PR. Because i want to keep this simple (seems trying to handle cases like this and communicate it to the user in a straightforward manner would be complicated).

In this PR I just try to improve the UX for “normal” multisigs like wallet_multisig_descriptor_psbt.py and https://github.com/bitcoin/bitcoin/pull/29156 - I don’t think it can ever be the case that one input is signed but another is not, and I’d expect this to be true for vast majority of wallets where users are using bitcoin core’s gui to sign psbts

All that said, interested to see what reviewers with more experience than me think

BrandonOdiwuor commented at 10:15 am on August 20, 2024: contributor

Concept ACK

Improve user dialog when signing multisig psbts #832

Reviews