Fix bug where rescan doesn't do a full rescan #5774

No description provided.

I don't understand the motivation here. Rescan should not pointlessly scan blocks prior to key creation times. If there is some reason that creation times are being set incorrectly that must be fixed, not papered over with a bypass in rescan.

If you update the patch, please make sure the commit message explains the motivation for the change.

I guess there's two issues: the first being I think when someone starts bitcoin with -rescan it should scan every block, as that is the safest behaviour and the one of least surprise.

The second issue, which I guess is not addressed by this pull request is someone does: bitcoin-cli importprivkey $PRIV_KEY "" false

and then restarts bitcoind (with or without -rescan) it will only start scanning at the earliest key creation time, which may very well be after the key was used. Bitcoin should probably persist something into the wallet letting it know it needs to rescan from block 0, but I'm not familiar enough with the code to do this.

As with many wallet behaviors, it is not documented (nor tested) what the behavior should be, so people give their own interpretation what it should be and try to fix it according to that. Not sure how to go forward here as there is clear disagreement.

I don't think it makes sense to assume the user wants to rescan from block 0 on next restarts, eg people import keys that are newly generated and don't need rescans at all.

I guess two things to support my interpretation:

• If I want to do a full scan (bad address seen date, importing without scanning), right now I'm forced to go bitcoin-cli importprivkey $RANDOMPRIVKEY to force the wallet to do a full scan. But bitcoind -rescan doesn't work. Strange, no?
• When running bitcoind -rescan the logs even say that it's scanning from the genesis block: https://github.com/bitcoin/bitcoin/blob/425473c056424574f73935add3f110fbce770273/src/init.cpp#L1229 which to me would indicate, that the intention of rescan is to actually rescan everything.
• I believe I might have already lost some coins due to bitcoind's behaviour. My usage pattern has been this: bitcoin-cli importprivkey $PRIVKEY1 "" false bitcoin-cli importprivkey $PRIVKEY2 "" false ...for all dozens of private keys I have..

and then finally:

bitcoind -rescan

The only reason I caught it this time, was because I knew i was missing a sizable amount of coins.

I don't think it makes sense to assume the user wants to rescan from block 0 on next restarts eg people import keys that are newly generated and don't need rescans at all.

Those people don't run with bitcoind -rescan

The -rescan option really shouldn't exist at all. It mostly only remains as historical inertia. The ability to run it has a marginal disk cost of 30+GBytes. Keeping it around has (apparently!) allowed serious birthdate tracking bugs to persist.

In any case, the interface is never obligated to perform some behavior which should be indistinguishable from the perspective of the user. I think the behavior you're looking for would be one to wipe out the birthdays, not a change in rescan behavior.

Agree with @gmaxwell.

• If you need behavior that clears the birthdays, add an option to clear the birthdays.
• If you need importprivkey to be able to set the birthday for your imported key, add an argument to importprivkey with the birthdate. This makes it possible to distinguish importing a new key and an old key, and tells the client from when to rescan

We're not trying to deny that there is an issue here. Right now importprivkey is ambiguous. We just don't agree on this fix.