chainntfns: Panics during initial sync blow up consumers.

[JoeGruffins]

Specifically, if wallet is locked during recovery btcwallet panics. Trying to use the wallet blows up whatever app is using it if a user simply locks the wallet at the wrong time. I also don't think there is a way to check if btcwallet is currently doing recovery.

I wonder if it's possible to remove the panic logic in *Wallet.handleChainNotifications altogether and handle errors more gracefully?

The point of panic that is causing some pain https://github.com/btcsuite/btcwallet/blob/27f244e8454076f5afcc5be6266c3762f902800f/wallet/chainntfns.go#L115-L119

The problematic recovery that we can not currently escape unless shutdown. Locking the wallet here panics. https://github.com/btcsuite/btcwallet/blob/27f244e8454076f5afcc5be6266c3762f902800f/wallet/wallet.go#L665-L792

[Roasbeef]

Ah, this makes sense, much of the recent work we've done in btcwallet was motivated by lnd. For lnd, the wallet is unlocked once at start up, then never locked again.

What's missing here is that we properly thread through a context.Context (or something like that), so it can be cancelled to ensure we can do things like unsafe shutdown.

I agree that panic doesn't really need to be there either, it should be handled gracefully. So we'd either need to try to retry once the wallet is unlocked again, or move it to something periodic tat attempts to sync again once things are clear.

[chappjc]

I wonder if it's possible to remove the panic logic in *Wallet.handleChainNotifications altogether and handle errors more gracefully?

This would be good. There are numerous ways this explicit panic in handleChainNotifications can be triggered:

• https://github.com/btcsuite/btcwallet/issues/729 (getblock json response unmarshalling)
• https://github.com/decred/dcrdex/issues/1690 (wallet db closed/shutdown)
• https://github.com/btcsuite/btcwallet/issues/687 (bad json payload in request)
• requesting a block from a peer failed (next comment) - couldn't retrieve block <hash> from network
• locking the wallet, as in this issue

[chappjc]

Just got this recovering a mainnet wallet:

panic: unable to synchronize wallet to chain: unable to perform wallet recovery: couldn't retrieve block 00000000000000000004d901bfc6973cad6a39d65f88ac6af77c2c212d1635fd from network

goroutine 4830583 [running]:
github.com/btcsuite/btcwallet/wallet.(*Wallet).handleChainNotifications(0xc0050d6900)
	github.com/btcsuite/[email protected]/wallet/chainntfns.go:117 +0x1065
created by github.com/btcsuite/btcwallet/wallet.(*Wallet).SynchronizeRPC
	github.com/btcsuite/[email protected]/wallet/wallet.go:213 +0x25c

The neutrino log had no direct error at the time, but shortly prior the node to which that getdata block request was made had dropped and reconnected a couple times.

[JoeGruffins]

This seems to come up a bit while testing dcrdex. Will attempt to stop the panics as simply as possible if that's alright. Will look through the code some more and come up with a plan.