btc spv: fix wallet birthday #1249
Conversation
client/asset/btc/spv.go
Outdated
| case noteI := <-notes: | ||
| switch note := noteI.(type) { | ||
| case chain.FilteredBlockConnected: | ||
| reportTip := atomic.LoadInt64(&lastReportedTip) | ||
| if int64(note.Block.Height) < reportTip { | ||
| w.log.Errorf("neutrino sent block notification after polling had already seen and sent the tipChange") | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm going to tear this lastReportedTip stuff out. I just wanted to have it here for discussion. I do see a ton of these messages printed on initial testnet sync, but I haven't ruled out that it's just this loop getting backed up. The other evidence for this in the wild is that refunds have been observed apparently taking an extra block to be reflected in the wallet balance. Similarly, after a restoration from seed with the fix in this PR, some funds I deposited into my testnet wallet days ago didn't appear until an additional block was mined. Checking the logs, there were a lot of neutrino sent block notification after polling... error messages for the last blocks scanned during the initial sync.
client/asset/btc/btc.go
Outdated
| @@ -473,6 +474,12 @@ func (d *Driver) Info() *asset.WalletInfo { | |||
| } | |||
|
|
|||
| func init() { | |||
| var err error | |||
| walletBirthday, err = time.Parse(time.RFC822, "21 Oct 21 18:01 CDT") | |||
There was a problem hiding this comment.
Sure you won't want to make this a few weeks earlier? PR open and PR merge are both pre-release anyway, so might as well keep our testnet wallets fully rescanable. I know I have one with a non-zero balance on this merge date.
That makes me wonder if we'll need a "rescan wallet" function. I'd be pretty surprised if we don't given how often that is a recommended action for wallets of all kinds. Reseeding the entire dex app would be a very inconvenient way to achieve this. The Rescan method would not even need to be part of the standard Wallet interface since it's not likely something we'd want to do with external wallets, just a bonus method we can detect by type asserting as a Rescanner interface.
Final thought on the subject is, Core should provide an export wallet seed function so users can recover funds outside of dex, assuming the external software follows the same derivation paths (but at least they could use other tools to derive keys for the correct path assuming we document it, and we should). I feel quite strongly about this despite it being a potential footgun for users, but it's no more of a risk than with external wallets and we've had that up until now. I think I'd probably make a cli app to take an app seed and asset ID to do this regardless.
Anyway, these rescan wallet and export wallet seed ideas are not relevant for this PR, just thoughts.
Begin wallet scan from the day the hd keys PR was merged, before any app seeds existed.