Sign PSBT: Can’t verify change output #732

1. 
   kallerosenbaum commented at 8:13 am on May 19, 2023: none

   Is there an existing issue for this?

   • I have searched the existing issues

   Current behaviour

   When I load a PSBT from file to sign it, the dialog that appears looks like this:

   

   There’s no way for me to know that the second output belongs to me (it does). Without knowing this I’d feel really uncomfortable signing the PSBT.

   Expected behaviour

   The change output should be marked as belonging to me.

   Steps to reproduce

   Open a PSBT file that has one output for the payee (not yours) and one output to you (as change)

   Relevant log output

   No response

   How did you obtain Bitcoin Core

   Pre-built binaries

   What version of Bitcoin Core are you using?

   25.0rc2

   Operating system and version

   Debian 11

   Machine specifications

   No response

2. 
   furszy commented at 1:29 pm on May 19, 2023: member

   Small note about this. This would work if the change address was previously recorded by the wallet (used in a previous tx). If you just obtained it by a getrawchangeaddress RPC call, the wallet will currently not detect it as change. Only will detect it as yours (reasons are explained in #25979).

   So, would say to present it as “own address” in the dialog instead of checking whether is change or not (external addresses should be presented as yours too).

3. 
   kallerosenbaum commented at 2:59 pm on May 19, 2023: none

   I’ll explain better what I do to trigger this, to make sure we’re talking about the same issue.

   1. Create a new wallet A on regtest, on an offline machine.
   2. listdescriptors on A and save the result
   3. Create a new blank wallet B on another machine
   4. importdescriptors on B using the result from step 2
   5. Get a new address from B and send coins to that address from another wallet
   6. In the GUI of B make a transaction to an external address, and make sure to have a change output
   7. Save the PSBT to file
   8. Open PSBT file it in the GUI of A.
4. 
   kallerosenbaum commented at 3:14 pm on May 19, 2023: none

   I don’t consider this as a mere inconvenience, but as a security issue. I can’t sign this transaction because I don’t know which of the outputs, if any, belongs to me. If people do sign these transactions, they’re going to be vulnerable to scammers.

   If the fix is hard to do, I think the “sign PSBT” feature should be removed from the GUI, as a security measure, until the issue is fixed.

5. 
   furszy commented at 3:31 pm on May 19, 2023: member

   My note was for other devs. Based on your comments, you want to get a visual distinction on addresses owned by the signer wallet in the dialog. Which is doable and not hard to do. My comment was just referring to the IsMine(addr) usage instead of the IsChange(addr). Which fulfills the required purposes in the same way (and also accepts external addresses which is more accurate).
6. 
   kallerosenbaum commented at 3:38 pm on May 19, 2023: none

   Got it, thanks!
7. hebasto added the label Wallet on May 19, 2023
8. bitcoin-core deleted a comment on Jun 10, 2023
9. 
   hernanmarino commented at 6:24 am on June 21, 2023: contributor

   Hey @kallerosenbaum , you are welcome to take a look at this PR I ’ve just uploaded to address your issue: #740
10. hebasto closed this on Jul 16, 2023

    ---

11. sidhujag referenced this in commit dc6b1250d4 on Jul 17, 2023
12. timemarkovqtum referenced this in commit 57b8336dfe on Jan 30, 2024
13. bitcoin-core locked this on Jul 15, 2024

Contributors
kallerosenbaum furszy hernanmarino

Labels
Wallet