Pay-to-self addresses don't display, can't be added to address book

[jamesob]

Found in master c948dc8f4285e27d8c0f3553d9d427b2a7b01b77

An address generated by the wallet for receiving isn't displayed by the address book and can't be added. When the user attempts to add it, an error message is presented saying 'The entered address "..." is already in the address book.'

I'm unsure if this behavior is unintentional or not, but it's pretty confusing.

Steps to reproduce

Generate a receiving address

Open the address book from the send interface. Note that the previously generated address is missing.

Attempt to add the previously generated address

Hit warning

[jamesob]

Looks like this is a consequence of the address being entered into mapAddressBook with purpose="receive" when the receiving address is created, and then filtering address book display for purpose="send" rows. We then don't allow the addition of duplicate addresses to the address book, even when the purposes differ.

One option is to simply not filter addresses for purpose when we're displaying the address book, e.g.

diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp
index f2ddbf259b..16f96abb73 100644
--- a/src/qt/addressbookpage.cpp
+++ b/src/qt/addressbookpage.cpp
@@ -40,11 +40,6 @@ protected:
     {
         auto model = sourceModel();
         auto label = model->index(row, AddressTableModel::Label, parent);
-
-        if (model->data(label, AddressTableModel::TypeRole).toString() != m_type) {
-            return false;
-        }
-
         auto address = model->index(row, AddressTableModel::Address, parent);

         if (filterRegExp().indexIn(model->data(address).toString()) < 0 &&

but that may get kinda dumb for cases when the number of receiving addresses in the book is large.

Another approach is to rekey mapAddressBook by std::pair<CTxDestination, std::string> where the string is purpose, but that's more work.

[jamesob]

The kind of repeated pay-to-self behavior that the address book would support probably is edge-casey and not recommended, so this may not be worth fixing.

[ryanofsky]

Maybe just clarifying the error message would be a good fix? I think

Can't add receiving address 2N3... ("ruh roh") to the sending address book

could be an improvement over "The entered address 2N3... is already in the address book."

One option is to simply not filter addresses for purpose when we're displaying the address book, e.g.

It seems like it would be confusing to show receiving addresses in the send dialog by default, but maybe there could be a checkbox giving the option to display them.

Another approach is to rekey mapAddressBook by std::pair<CTxDestination, std::string> where the string is purpose, but that's more work.

I don't think this would be great, because it would also allow associating different names with the same address. But one way you could allow addresses to be used for both sending and receiving would be to extend purpose field with a "receive+send" option. Or make purpose a set of strings instead of a single string.

[laanwj]

You're right - this is a design constrain due to how the address book is handled internally. It has always been the case and is not a bug:

• Sending addresses concern external addresses, to be sent to
• Receiving addresses are always your own addresses. The receiving address book contains all non-change receiving addresses.

There has been talk (in #7729) about supporting multiple labels per address (as a future idea). The same underlying infrastructure could be used to support this.

Also: "address book" has always been the wrong name for the functionality (it's inherited from Satoshi's original GUI). It's a list of historically used (sending/receiving) addresses, not intended to be re-used.

For the short term it's IMO best to fix the error message as @ryanofsky says, and regard this as expected behavior.

[jamesob]

Roger that. Thanks for the input, @ryanofsky @laanwj. I'll file a PR clarifying the error messages.