Should the GUI have a "you're sending again to the same place you sent before"?

gmaxwell commented at 12:34 pm on November 16, 2013: contributor

when you try to send to some place you’ve already sent, perhaps there should be a polite “You’ve already sent X btc to this destination in TXN y on date Z. Do you want to continue?” which should help people avoid some of the incidents they’ve had with double paying.

gmaxwell commented at 7:58 pm on November 17, 2013: contributor

Doubling payments is an actual mistake that people make which could be dampened by a polite confirmation, one which could be disabled with a don’t tell me again, that it also provides a hook to educate people about the consequences, both for themselves and for others, is just a bonus. The education matters, considering response I’ve had to bip32 public chains has been “why does it matter, I know who I’m paying already”. :)

MarcoFalke commented at 7:55 pm on September 27, 2016: member

It would also help to educate about address reuse.

CryptAxe commented at 2:04 am on February 4, 2017: contributor

Do either of you have any thoughts as to how to implement this? Looping through the wallet’s transactions and comparing the destination address to what the user wants to send to now seems possible, but maybe not the best approach?

luke-jr commented at 3:00 am on February 4, 2017: member

I set a label for every payment, so whenever I go to pay an address a second time, it already loads that label up… which I take as an implicit warning (and then I go complain to whoever gave me the same address twice). Making it an explicit warning sounds like a good idea, and can probably be done with some flag on the addressbook entry easily enough.

laanwj commented at 10:05 am on February 4, 2017: member

Indeed, searching the address book should be enough, which is much faster than searching the transactions.

CryptAxe commented at 10:44 pm on February 4, 2017: contributor

Alright heres a simple solution I am testing out. This will just check if the address you have pasted into the send coins entry address field exists in the address book already, and display a warning message if it does.

This will also display the error message if you create a receiving address from the request dialog and then try to send a payment there. I’m not sure if that is a problem because users probably aren’t doing that too often.

I also noticed that the label field is not cleared when a new address without a label is pasted, the UI displays the old label so I fixed that as well.

https://github.com/CryptAxe/bitcoin/commit/0698a278274c32b2b90901561d1f8f510cd4890a

luke-jr commented at 11:56 pm on February 4, 2017: member

If you type a label before the address, you don’t want it cleared.
Consider the scenario where a user gets a loan, and saves the repayment address in his address book for later. I would consider it a bug if he got a warning when he tried to send to it the first time. So some explicit flag is needed (it can be set on the address book entry went it is used for sending).
“Address reuse is not recommended!” is too soft and unclear. I suggest “You have already paid this address!”, and making it a hard error with a manual override (dialog with an “Ignore and send anyway” option?)

CryptAxe commented at 10:55 pm on February 5, 2017: contributor

Thank you for the feedback. In this new branch I made it so that an address will be marked as used when it is sent to via UI. If the user tries to send to that address again, a warning message will popup and require that they accept the consequences to continue. If multiple recipients are entered, each will be checked and the user will need to verify that they want to reuse each one before the send tx confirmation dialog is shown.

I’m open to suggestions on the language in the popup message, but I copied a reddit comment from Luke which I believe sums it up nicely.

https://github.com/CryptAxe/bitcoin/commit/44d74135ed7c18e43691b6d6c017574a86846ee5

CryptAxe commented at 11:00 pm on February 5, 2017: contributor

luke-jr commented at 11:12 pm on February 5, 2017: member

Just add a new key to destdata?

CryptAxe commented at 11:41 pm on February 5, 2017: contributor

Handling tagging the address as used in the same way the labels are done just made more sense to me. This way would make it easier, I think, to add whether or not the address is used to the address book view as well (just a few lines would need to be added). I hope to update the address book view to be search-able and filterable in the near future and being able to show only unused / used addresses would be a useful filtering option.

The loop looking up destdata and the loop I wrote to lookup the used value are basically identical with the destdata loop being slightly more complicated but probably no real performance difference. I can change it still though of course if I am missing something that makes the way I did it worse. Thanks!

luke-jr commented at 1:58 am on February 6, 2017: member

AFAIK the destdata exists specifically for extending the address book like this.

CryptAxe commented at 3:55 am on February 7, 2017: contributor

The address table model actually doesn’t even know about the destdata as far as I see. It would be possible to add the used value to destdata, but it would be a lot more work to add that to the address book view. I still think adding the used state to the address model is the way to go, maybe this will convince you:

https://github.com/CryptAxe/bitcoin/commit/dc347b30e5c76b0a7e66c935a466fd03b612a187

luke-jr commented at 5:28 am on February 7, 2017: member

No reason AddressTableEntry can’t use destdata to get the used flag?

CryptAxe commented at 9:16 pm on February 7, 2017: contributor

Alright I give up and will do it your way :+1: Do you have any feedback on the popup message text or anything else before I make the “final” draft?

luke-jr commented at 9:25 pm on February 7, 2017: member

0tr(
1    "You have sent to the Bitcoin address %1 already!\n\n"
2    "Sending to the same address twice is technically not supported and will reduce your own security as well as the privacy of all Bitcoin users."
3).arg(address) + "\n"

CryptAxe commented at 6:18 am on February 8, 2017: contributor

https://github.com/CryptAxe/bitcoin/commit/9113784bd6055e51703306a23fbcefba343f018a

luke-jr commented at 6:32 am on February 8, 2017: member

Looks more or less good enough to open a pull request at least.

CryptAxe commented at 6:47 am on February 8, 2017: contributor

Thanks for helping!

Sjors commented at 7:35 pm on March 16, 2018: member

There was a PR last year, but closed due to inactivity.

Concept ACK.

promag commented at 9:46 pm on July 27, 2018: member

Related to #13756.

MarcoFalke removed the label Refactoring on May 9, 2019

promag commented at 4:42 pm on March 4, 2020: member

Doubling payments is an actual mistake that people make @gmaxwell this case is different because both the destination and the amount must duplicate to be considered double payment. I consider this case orthogonal - the message should warn it is a double payment and maybe an option to see the last payment.

Bushstar referenced this in commit 4b7c175f53 on Apr 8, 2020

fanquake commented at 8:27 am on August 8, 2022: member

Moved to https://github.com/bitcoin-core/gui/issues/648.

fanquake closed this on Aug 8, 2022

bitcoin locked this on Aug 8, 2023

Should the GUI have a “you’re sending again to the same place you sent before”? #3266