Improve Wallet and LDK node business logic handling #113
Conversation
# Conflicts: # app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt
|
I tried to make the refactor as specific as possible, I'm open to suggestions to this PR and further refactors |
jvsena42
changed the title
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
Almost ready IMHO, super nice work, thank you!
Please check review comments & address.
tests refactor:
- launch with existing wallet - 🟢
- wipe wallet - 🟠 UI freeze while wiping for a little while
- create new wallet - 🟢
- deposit sats & mine - check balance & empty views - 🟢
- restore wallet - check UI & restore functionality - 🟢
- cjit - scan & pay while app is in bg - bg receive - 🟢
- try to break app with quick minimise + restore - 🟠 discussed in Slack
- send LN & onchain, receive LN & onchain - 🟢
| lightningRepo.getPayments().onSuccess { | ||
| coreService.activity.syncLdkNodePayments(it) | ||
| syncState() | ||
| }.onFailure { e -> |
There was a problem hiding this comment.
This wasn't and isn't working correctly on app returning to FG, but at least now we know, thanks to the error logging 👍🏻
The actual issue is in the timing, it gets called before the node is ready.
<just info, can resolve comment>
There was a problem hiding this comment.
I can improve this in the next refactor. I'll probably wrap most lightningRepo in a waitForTheNodeStart(callback: () -> Unit) method
|
Thanks for the good points in the review! |
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
Thanks a lot for addressing all review comments 🫡
Looks good, very nice refactoring I must say, it was long due 🥇
Ran a few sanity checks again, everything still works, I'm merging this 🚀
This PR improves the Wallet and LDK-node business logic handling, preventing crashes due to LDK-node / channels not initialized.
I tried to separate in two PRs but it wasn't possible due to coupling in the WalletViewModel.
Related to #50