TurboSync: Remove an extra while

[kiminuo]

See #10948 (comment)

[adamPetho]

If we remove it, how are we exiting the while (true) loop? LOC274

[kiminuo]

cancel.ThrowIfCancellationRequested();.. it should be while (cancel.IsCancellationRequested) really. The loop should continue looping after Wallet.StartAsync() is callled until Wallet.StopAsync() is called.

It would be actually good to add a test for Wallet.PerformFinalSynchronizationAsync but it is not exactly trivial with all those dependencies. Maybe it's doable though.

[turbolay]

Why don't remove the while instead? I think that was the idea of 5ea5805

[kiminuo]

Honestly, looking at the code, I'm more and more confused. I thought that PerformFinalSynchronizationAsync is a long-running task but then using (await HandleFiltersLock.LockAsync(cancel).ConfigureAwait(false)) would be hold for a really long time.

@turbolay: You know the design better than me.

[molnard]

I believe the while makes no sense here - it is a leftover. In case of exception, the while won't retry. In case of success, the while won't iterate either.

[kiminuo]

So the idea of PerformFinalSynchronizationAsync is that it does the synchronization once after the wallet is started?

[turbolay]

So the idea of PerformFinalSynchronizationAsync is that it does the synchronization once after the wallet is started?

Yes. Whether we should retry or not, catch the exception or not, was my final question that I asked here: #10896 (comment). @molnard took over after this

I thought that PerformFinalSynchronizationAsync is a long-running task but then using (await HandleFiltersLock.LockAsync(cancel).ConfigureAwait(false)) would be hold for a really long time.

This is true, the design for this is explained here: https://github.com/zkSNACKs/WalletWasabi/pull/10663/files#r1206981555
I will make a new PR to add comments on the usage of this lock

[turbolay]

LGTM

[molnard]

Remove cancel.ThrowIfCancellationRequested();

[molnard]

ACK