WalletWasabi / WalletWasabi

Open-source, non-custodial, privacy preserving Bitcoin wallet for Windows, Linux, and Mac.
https://wasabiwallet.io
MIT License
2.12k stars 498 forks source link

GUI evolves to a chameleon #10814

Open MarnixCroes opened 1 year ago

MarnixCroes commented 1 year ago

How To Reproduce?

press CTRL C shorlty at start up after building from source minimize/maximize doesn't change anything, cannot press close

The issue is related to that the shutdown after CTRL hangs a bit, it will stay at [1] INFO WasabiAppBuilder.BeforeStopping (94) Wasabi GUI stopped gracefully (c38579ba-e5b9-4fb5-964c-10e0193c0f6c). before the program really quits

Screenshots

Screencast from 30-05-2023 11:43:13.webm

Operating System

deb

Wasabi Version

325db12a33ec665202029088754e27ba1f57accc

MarnixCroes commented 1 year ago

Here the same, but after minimizing, the GUI pops back up but without issues. Which is interesting as Wasabi did a restart by itself.

Screencast from 30-05-2023 11:47:36.webm

soosr commented 1 year ago

press CTRL C shorlty at start up after building from source

Can you reproduce the issue when normally close the software? Can you reproduce the issue when building from source, but in release mode? Can you reproduce the issue when NOT building from source?

MarnixCroes commented 1 year ago

@soosr

Can you reproduce the issue when normally close the software?

no

Can you reproduce the issue when building from source, but in release mode?

yes

Can you reproduce the issue when NOT building from source?

no

kiminuo commented 1 year ago

@soosr I think we should add a cancellation token checking here:

It does not fix this issue but it looks to me like a similar issue as @MarnixCroes reported. And even if it were not fixing anything, it's just plain wrong to use that while + Task.Delay. It's just a matter of time before it backfires.

soosr commented 1 year ago

I don't think the cancellation token is the solution we should go to. It's the UI side, where even the Task.Delay smells. The logic should be rethought and should be "event based". By even based I mean every trigger should have a sign that the UI can reflect on.

  1. Wait until the backend is connected (or get in error and do the fallback). Current code for it
  2. Get the values for calculating the estimation. Current code for it
  3. Start updating the estimation. Current code for it
  4. An event when BitcoinStore.SmartHeaderChain.HashesLeft is 0, which means the wallet has all the filters and wallet loading can be started. Basically, this event would replace the code where you want to add the cancellation token.
  5. Call wallet loading method.
molnard commented 1 year ago

privacy maxed out.

kiminuo commented 1 year ago

FTR, this was fixed here https://github.com/zkSNACKs/WalletWasabi/pull/11324/files#diff-4b4a724533ee24d0be13a268e6eaf9e1c3b6f1cc85980a6ace26007d5d14d0a3L98-L101.

This is the next step.

Pule08 commented 1 year ago

@MarnixCroes could you please check, could you reproduce it, or not?

MarnixCroes commented 6 months ago

@MarnixCroes could you please check, could you reproduce it, or not?

yes I can, 87f2521a17a80c98582484b72303bd72e74f44b9 but only shortly, like a few seconds. it's not as OP

soosr commented 5 months ago

@MarnixCroes

but only shortly, like a few seconds.

So it works after a few seconds?

MarnixCroes commented 5 months ago

@MarnixCroes

but only shortly, like a few seconds.

So it works after a few seconds?

Yes it shutdown after the few seconds