Allow cache purging at wallet initialization

[AdamISZ]

Fixes #1614.
Prior to this commit, if data in the persisted cache in the wallet file were wrong (should be a very extraordinary case), then the joinmarket code would have to crash with a cache invalid warning. After this commit, in such an extraordinary case, the option exists to invalidate or remove the cache on startup, so that it can be rebuilt from scratch. This is done with a config var wallet_caching_disabled in the POLICY section.

[AdamISZ]

I will comment that I spent quite a lot of time figuring out a model for this that didn't have side effects. You can't just naively delete the cache at the moment when you start sync_wallet in WalletService; the issue is that in the initialization of the Wallet object, we read the cache from disk and build the address and script maps (see _populate_maps), and it's those maps which feed into syncing (see is_known_addr) - which is somewhat work-aroundable, except that we've designed the --recoversync option to be something that is sometimes done repeatedly in a loop. Although it seemed more elegant, it proved to be less so, to isolate this to being a decision at the time of syncing the wallet.

So after a few ideas, I settled on putting it into a config var, with a warning that you generally DO NOT want to use it. That way the change in loading from storage (_load_storage method) can be isolated to specifically the open_wallet method that encapsulate the **Wallet constructor(s). So syncing repeatedly in a loop is fine, as we still only call the actual Wallet constructor(s) once.

The test case is very simple but was enough to show these issues, so it kind of already did its job ...

[kristapsk]

Shouldn't --recoversync always enforce this and to jm_single().config.set("POLICY", "wallet_caching_disabled", "true") at startup?

[AdamISZ]

Shouldn't --recoversync always enforce this and to jm_single().config.set("POLICY", "wallet_caching_disabled", "true") at startup?

So to expand on what I said above, I came to the conclusion that probably not. (I know, this is what we both agreed earlier).

First, recoversync goes in a loop and will often run the sync algo several times. This doesn't sit so well with the fact that we'd have to rebuild the cache every one of those times. It also makes the code messier (as somewhat alluded to above).

Additionally, this is a pretty special situation - if the cache got screwed up, it would almost certainly have to be a bug in the code, and a really bad one at that. Hence I feel like, if a person found that something was horribly wrong with their wallet syncing process, and they didn't want to start from pure zero (let's say, from seed, recover, new Core node even - in which case this option is not helping them) - maybe because their wallet is huge? - then, this option exists. But this case is very exceptional and probably won't happen.

[kristapsk]

cr utACK, code seems correct. But see type hint nit comments above.

[kristapsk]

utACK ef1d76e