Keys and addresses don't have balances. You're not supposed to use import/dump privkey like this. Just backupwallet for backups.

Though I agree with Luke's opinion here, it's not an excuse for broken functionality. If you're using dumpprivkey/importprivkey, you're already managing your wallet at the per key level, and as we have rpc methods for that, they should work.

Agree with @luke-jr here. What you are doing is very dangerous. You should never assume that the balance is only on one key. Many people lost their coins this way.

This is exactly what dumpwallet/importwallet is for which is in master (and will be in 0.9). These methods export and import all the keys in your wallet.

Code changes look ok to me though...

OH, this probably explains some misbehavior I saw with salvagewallet missing outputs! See, look at how much easier slacking off and not investigating the cause was…

Can we get some ACKs/NACKs here? This is a very simple patch, and if it solves a problem (and in the correct way) it should be in 0.9 IMO.

This is not the correct way to solve the problem, IMHO.

We should set pwallet->mapKeyMetadata[keyid].nCreationTime to 1 in importprivkey, before calling AddKeyPubKey, instead of initializing every key's birthdate to 1.

I do agree that import/dump need a way to deal with birthdates, though. I suggest adding a bool to dumpprivkey [includebirth=false], which if set, outputs a string privkey@unixtimestamp. Importprivkey can then support keys in that format too, and set the imported key's timestamp to the provided one, instead of 1.

I have now updated to what @sipa suggested.
Tested both, rescanning through importprivkey and through -rescan.

About the import/dump of birthdate:
I dont like so much bothering the user with the birthdates, I think this should be transparent to the user.
So how about introducing a version parameter to dumpprivkey [version=2].
So default is version 2, dump with @unixtimestamp. Now if someone needs to export from 0.9
and import in 0.8, he can call dumpprivkey version=1, exporting without @unixtimestamp.
Good/bad idea?

Also maybe adding a little checksum at least to the timestamp would be good, I think, as importing a private key should be guaranteed to work. If checksum wrong, set timestamp to 1.

@cozz I consider this function micro-management. We provide it, so it must work. But in general, I'd very much prefer to encourage entire wallet dumping/importing (dumpwallet/importwallet) which already handles birthdates. I wouldn't bother adding a checksum there too.

Regarding a version number rather than a boolean... meh, I don't think we'll add many more versions later for the same typo of keys.

Haven't tested, but ACK code changes.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1f12844fc03dfc1863ea9a5096f8650c64a0b2ac 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.

ACK (much easier to review with this localized change)