TurboSync: Follow-up ideas about walletwasabi HOT 13 OPEN

The suspicious while was changed by @molnard in this commit: 5ea5805

Maybe some kind of leftover? I believe the thing to do is to remove it.

from walletwasabi.

Probably pls do it

from walletwasabi.

This is mysterious indeed, let's find out who was the murderer.

where the while is suspicious as there is also break.

The while makes no sense - it is left over by me. I assumed that if an exception is thrown here it is a fatal one - we won't try anything again.

The case of P2P interaction throwing an exception

Not all BlockProvider.GetBlockAsync makes sense to retry. Retrying should be done on a lower level. If it throws that means there is no way to get the block.

from walletwasabi.

The case of P2P interaction throwing an exception

Not all BlockProvider.GetBlockAsync makes sense to retry. Retrying should be done on a lower level. If it throws that means there is no way to get the block.

My concern here is only:

1. You start WW and log in to your wallet
2. A block cannot be downloaded because, say, your Internet connection is down.

Now what does happen next? Do we recover from this once the Internet connection is up again? I guess we will perform synchronization as usually. Is it the idea?

from walletwasabi.

I'm concerned by "doing nothing" in case of an exception because failing to get a block would mean releasing HandleFiltersLock so new filters will be processed on all keys without finishing the synchronization.

from walletwasabi.

The only way to have an exception right now is when RemoveAndStopAllAsync from the WalletManager is called

So this is not really an issue. There is no other way because the P2PBlockProvider never returns null, so TryGetBlockAsync never returns null when GetBlockAsync is called from the SmartBlockProvider, therefore GetBlockAsync can't throw.

from walletwasabi.

Do we recover from this once the Internet connection is up again?

No. The wallet sync stops at that point and will never continue again.

Is it the idea?

There is no current idea for this case. We put a log message, that is all.

from walletwasabi.

I'm concerned by "doing nothing" in case of an exception because failing to get a block would mean releasing HandleFiltersLock so new filters will be processed on all keys without finishing the synchronization.

What if we go to an infinite loop until the sync can be finished actually? Maybe this was @turbolay your original intention?

from walletwasabi.

Maybe this was @turbolay your original intention?

Yes this was my original solution.

from walletwasabi.

@turbolay can you add that back?

• Keep the design of PerformFinalSynchronizationAsync cannot throw just log - this is to avoid UnboservedTaskException.
• Add an infinite loop around PerformWalletSynchronizationAsync.
  • Consequence: in case of an error we won't be able to process new filters but the wallet cannot get into a broken state.

WDYT?

from walletwasabi.