Address Book related Fixes

[sgimenez]

This is a rebased series of patches from pull request #335

1. Fix the synchronization of sending addresses between a CWallet and its associated CWalletDB (this was reported independently in Issue CWallet: fix mapAddress book update #350).

2. Add a check for validity of sending addresses (Issue Validity of sending addresses in the Address Book. #328).

3. Avoid propagation of unnecessary updates to CWalletDB.

4. Fix the behavior of setaccount on sending addresses that are already listed in the Address Book. (Issue Using setaccount on addresses that are already listed in the Address Book. #329).

5. Add wallet methods GetDefaultAddress and SetDefaultAddress (contributed by laanwj in Issue CWallet: fix mapAddress book update #350).

[sipa]

1. ACK
2. ACK; I don't like using mapPubKeys as a check for whether a key is ours though (there's pwalletMain->HaveKey()), but for now it's correct.
3. ACK
4. NAK: you change the check for whether the key belongs to a certain account, but still look it up there the next line?
5. I like accessor functions for those things, but a GetDefaultAddress should return the address corresponding to vchDefaultKey, and SetDefaultAddress should update vchDefaultKey (and push to db), imho.

[sgimenez]

Ok i've amended the commits.

About 4), I assumed previously that mapPubKey contained a subset of mapAddressBook. Maybe it is (or will become) false, but anyway it is better to add the check.

About 5), I hope I've done the wanted changes.

About HaveKey(), there are many other occurrences of mapPubKeys that should be replaced as well.
So it's probably better to leave it unchanged until a global substitution is performed.

[sipa]

mapPubKeys is not a subset of mapAddressBook; it contains reserve keys while mapAddressBook doesn't. You can assume for now that mapPubKeys does contain all 'our' keys though, and ignore HaveKey for now.

[sgimenez]

Indeed. Reserve keys!
Thank you.

[jrmithdobbs]

Can we get a breakdown of the original problems this addresses instead of just what was done to correct them?

[sgimenez]

1. If you need a concrete appearance of the underlying problem, make some changes in your address book using the gui (current head), close the address book window, open it again. Changes are not displayed.

2. and 4) see respective links.

3. and 5) are just improvements.

[sipa]

ACK.

[TheBlueMatt]

NACK - crashes when loading from empty .bitcoin - "Error to load wallet.dat"

[TheBlueMatt]

Problem first showed up in 0efc5d29a2d8922b10dcd02a1f1ad7b46258edd8, Ill let you do the rest.

[sgimenez]

Yes a small issue with big consequences.
The address name "" was generated as the default "map"ed value, and therefore it was not written to db.

Note to myself, I should really keep away from languages that allow me to make such stupid mistakes :-)

[sgimenez]

Status update.

1. has been pulled.

2. and 4) are still pending fixes for outstanding bugs.
   Note: expect merge conflicts with recent work by sipa.

[sipa]

If 1 has been pulled, why is its commit still listed here? Can you do a rebase, removing the already-merged parts?

[sgimenez]

Dunno, the first patch was kind of cherry-picked.
Here comes the clean rebase.

[sgimenez]

Rebased against recent changes. Removed 5) since its purpose is no so clear now.

[sipa]

How relevant are these fixes still?