search

decred / dcrwallet

A secure Decred wallet daemon written in Go (golang).
https://decred.org
ISC License
216 stars 155 forks source link

wallet: Remove mainchain tip check on NeedsAccountsSync #2327

Closed matheusd closed 8 months ago

matheusd commented 8 months ago

This check could prevent a wallet from going through address discovery if it had not completed the initial chain sync prior to being restarted.

One instance this could happen is when restoring SPV wallets under an instable network connection or having a power loss during the initial sync of a restored wallet.

Detected while debugging issues with #2318.

jrick commented 8 months ago

I'm pretty sure i've relied on this as a hack to get around the initial discovery when i planned to avoid it anyways by manually setting the address indexes, e.g. on a voter with mixed ticketbuying and large gaps.

Seems fine to fix this, but I do think we want a proper way to intentionally opt out, probably with some additional prompts during wallet creation or more flags.

jrick commented 8 months ago

Wait, hold on. This is for account discovery, not address discovery.

The only location this is used in dcrwallet itself (not importing an embedded wallet) is https://github.com/decred/dcrwallet/blob/master/dcrwallet.go#L485-L500, and all this does is cause the wallet to be unlocked at the time of discovery (it will run regardless of what this function returns or not), and because the wallet is unlocked, it will opportunistically also discover accounts and not just addresses.

matheusd commented 8 months ago

Seems fine to fix this, but I do think we want a proper way to intentionally opt out, probably with some additional prompts during wallet creation or more flags.

You mean, opt out of address/account discovery on a given wallet? Something like --noaccountdiscovery and/or --noaddressdiscovery? If so, I can these on this PR.

jrick commented 8 months ago

They aren't needed on this pr, but yeah flags like that would be nice.

This still looks like a bug worth fixing as is, but I just don't think the explanation is accurate. Is some other software e.g. dcrlnd using this return result to trigger address discovery?

matheusd commented 8 months ago

No, you are correct this is regarding account discovery, not address discovery. I'll fix the description in the commit.