GUI: Receive: Remove option to reuse a previous address #3716

luke-jr commented at 4:44 AM on February 21, 2014: member

This was justified by the need to "resent" an invoice, but now that we have the request history, that need should be gone.

BitcoinPullTester commented at 5:33 AM on February 21, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1b7f5112a53bd24709ed79bf239b5820c5488437 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj commented at 6:51 AM on February 21, 2014: member

I added this option on purpose because I don't want to lose the older functionality of generating a QR code for previous addresses (which are not longer in the recent history).

IMO removing it would be blunting a tool just to avoid people cutting themselves in the foot on purpose privacy-wise.

luke-jr commented at 6:56 AM on February 21, 2014: member

@laanwj How about importing all previous "address book" entries into the requested payments history?

laanwj commented at 8:06 AM on February 23, 2014: member

Meh, that would import all people's previous addresses (which can be a lot for old wallets) as ugly payment requests with missing fields.

laanwj commented at 10:37 AM on March 16, 2014: member

Closing this for now, could be reopened at some point when deterministic wallets are implemented (then there is no excuse left to reuse addresses).

laanwj closed this on Mar 16, 2014

luke-jr commented at 7:32 AM on February 11, 2017: member

We have HD wallets now. Should this be reopened?

laanwj reopened this on Feb 11, 2017

luke-jr force-pushed on Feb 17, 2017

luke-jr commented at 6:30 AM on February 17, 2017: member

Rebased

fanquake added the label GUI on Feb 19, 2017

da2ce7 commented at 8:08 AM on February 20, 2017: none

I don't think that the feature of [reusing previous address] is needed if you have a HD wallet. However, should we still provide this option for those who have old wallet?

jonasschnelli commented at 1:51 PM on March 17, 2017: contributor

IMO the address reuse function should only be removed then CWallet::IsHDEnabled().

jonasschnelli commented at 9:22 AM on April 19, 2017: contributor

Any plans to reduce the feature to HD wallets only?

TheBlueMatt commented at 4:51 PM on July 11, 2017: member

Agree with @jonasschnelli, though I'm happy to do this anyway if we add an -upgradewallet for HD wallets and wait a release or two.

laanwj commented at 1:17 PM on October 2, 2017: member

Not sure. I think if we remove this feature, it should be removed wholesale. Otherwise this code will likely sit untested and rot (there's no automated test for GUIs and everyone tests with new wallets...).

in src/qt/receivecoinsdialog.cpp:132 in e1b7c7343f outdated

 128 | @@ -130,22 +129,7 @@ void ReceiveCoinsDialog::on_receiveButton_clicked()
 129 |  
 130 |      QString address;
 131 |      QString label = ui->reqLabel->text();
 132 | -    if(ui->reuseAddress->isChecked())
 133 |      {

TheBlueMatt commented at 10:26 PM on October 3, 2017:

Wanna just kill this scope?

TheBlueMatt commented at 11:55 PM on October 3, 2017: member

Gah, I missed the context, really sorry about that. Yea, I'd say just remove it - you can already do the much more intuitive double-click on existing entries in the "Requested payments history" section and get the QR code for old payments, plus that way it only lists things that you "Requested", not all your random receive addresses in the popup for "Reuse"

Tested ACK (a rebased-on-master version of) e1b7c7343f1d7f845e36a5aefda3db6056a7c350

meshcollider commented at 2:14 AM on October 4, 2017: contributor

Concept ACK

MarcoFalke assigned jonasschnelli on Nov 10, 2017

MarcoFalke commented at 9:42 PM on November 10, 2017: member

ACK e1b7c7343 (see screenshots)

Note that it is still possible to regenerate QR codes of addresses where a QR code was previously generated. However, it is not possible to generate QR codes for addresses which were generated by rpc (getnewaddress et al.)

I think that is fine.

Before: screenshot from 2017-11-10 16-31-13

After: screenshot from 2017-11-10 16-31-13

luke-jr commented at 6:26 AM on November 11, 2017: member

(If we want QR codes for past addresses in general, we could possibly add a Show QR Code to the address book)

jonasschnelli commented at 6:43 AM on November 11, 2017: contributor

utACK e1b7c7343f1d7f845e36a5aefda3db6056a7c350 @luke-jr: can you please remove the unused scope to make this PR ready for merge? https://github.com/bitcoin/bitcoin/pull/3716/files#r142540439

GUI: Receive: Remove option to reuse a previous address

This was justified by the need to "resent" an invoice, but now that we have the request history, that need should be gone.

927f4ff5a2

luke-jr force-pushed on Nov 11, 2017

luke-jr commented at 7:51 AM on November 11, 2017: member

Done

promag commented at 8:28 AM on November 11, 2017: member

utACK 927f4ff5.

MarcoFalke commented at 6:11 PM on November 11, 2017: member

MarcoFalke merged this on Nov 11, 2017

MarcoFalke closed this on Nov 11, 2017

MarcoFalke referenced this in commit 13e352dc53 on Nov 11, 2017

PastaPastaPasta referenced this in commit e272cee292 on Jan 17, 2020

PastaPastaPasta referenced this in commit 2719d2c45f on Jan 22, 2020

PastaPastaPasta referenced this in commit abc0fb08be on Jan 22, 2020

PastaPastaPasta referenced this in commit 444f6945be on Jan 29, 2020

DrahtBot locked this on Sep 8, 2021