thunderbiscuit / padawan-wallet

The bitcoin wallet trainer on Android.
https://padawanwallet.com/
Apache License 2.0
111 stars 49 forks source link

Fix: Generate address qr in suspended thread #279

Closed yellowHatpro closed 1 year ago

yellowHatpro commented 1 year ago
darkvoid32 commented 1 year ago

Hi @yellowHatpro , would clicking the Generate a new address button launch multiple computations of a new QR on separate threads? In that case we might want to either: 1) Add a flag to ensure only generate 1 QR at a time, or 2) disable the button while the QR is being generated. What do you think?

yellowHatpro commented 1 year ago

As per my fixes, I am calling the QR-generating function inside a Launched Effect (and inside the IO thread), whose key is the address itself. So if the address is changed, it will then only prompt the QR-generating function (only once) to be called again. I didn't investigate the Generate a new address button yet, but I didn't notice anything to change even if I pressed the button. Yeah, we can do something like a progress bar while generating the new QR; what say???

darkvoid32 commented 1 year ago

Oh sorry for not clarifying, I was referring to issue #286 . Just in case multiple threads are created when users press the generate qr code more than once here and something unexpected happens.

I think adding a spinning circle might be a good feature to have, and you can open up a new issue (and pr) for that!

yellowHatpro commented 1 year ago

Aaaalright I am on it 🚀😁

darkvoid32 commented 1 year ago

Thanks, so just to confirm again does pressing the Generate a new address button create multiple threads? If that happens it would be good to add a flag to stop multiple threads from being created!

yellowHatpro commented 1 year ago

Hi, I don't see anything happen when I click the Generate a new address button. I don't know what the purpose was, but the function (getLastUnusedAddress) always returns the same value.

yellowHatpro commented 1 year ago

So I think no new threads are spawned on clicking the button

yellowHatpro commented 1 year ago

And also, as per the current implementation of the Generate new address button, I think even if I use a progress bar to appear when the button is clicked and QR is generated, there won't be any effect because nothing actually happens on clicking the button.

darkvoid32 commented 1 year ago

Looks good to me! Sorry for not catching it earlier, could you quickly rename the commit to our convention, something like the PR name, i.e. Fix: Generate address qr in suspended thread

yellowHatpro commented 1 year ago

@darkvoid32 I've changed the PR and Commit name accordingly

yellowHatpro commented 1 year ago

I am not sure, but did you mean the commit message? Sorry I actually changed commit lol

darkvoid32 commented 1 year ago

Yeap I meant the commit message! Just merge all of them back into 1 commit and its good to go afterwards.

yellowHatpro commented 1 year ago

@darkvoid32 done senpaii✅