[WIP] Display 'Connecting...' when connection to daemon is lost #829
Conversation
j-berman
force-pushed
the
fix-dropped-cxn
branch
from
42d5a3b to
4ac01ab
Compare
| } | ||
|
|
||
| public void onStartRescan() { | ||
| unsync(); | ||
| firstBlock = 0; |
There was a problem hiding this comment.
i am not sure not unsetting firstBlock in unsync() has no adverse effects
There was a problem hiding this comment.
Is there another place unsync() is called I'm not seeing except in the place I added? Or do you mean that it should also unset the firstBlock even in that place I added the call to unsync()?
There was a problem hiding this comment.
Will dig into it/test it some more. The issue I was thinking with resetting it in that new call to unsync I added is if it might cause continual resets of the progress bar before it's synchronized IIRC
There was a problem hiding this comment.
i don*t see another place either. yes, i mean also unsetting firstBlock - maybe even scrapping this method as its only used once and has no obvious reuse scenario (or?).
| connectionStatus = Wallet.ConnectionStatus.ConnectionStatus_Connected; | ||
| lastDaemonStatusUpdate = t; | ||
| } else { | ||
| if (t - lastDaemonStatusUpdate > STATUS_UPDATE_INTERVAL) { | ||
| long statusUpdateInterval = wallet.isSynchronized() ? STATUS_UPDATE_INTERVAL_SYNCED : STATUS_UPDATE_INTERVAL_SYNCING; | ||
| if (daemonHeight == 0 || t - lastDaemonStatusUpdate > statusUpdateInterval) { |
There was a problem hiding this comment.
Also calling out this part. Notice how I added daemonHeight > 0 in the above if statement, and the first time updateDaemonState gets called, daemonHeight == 0, so it will fall into this if and make the request for the height. If the height is already cached, it won't be a problem.
The work over in monero-project/monero#8076 should minimize trips to the daemon and ideally get the height cached so the client shouldn't need to make a trip to the daemon here the first time this is called. Looking into this as well.
|
do you know if the required core changes be in the upcoming fork? |
|
Yep, required changes will be in |
m2049r
changed the title
The issue
When the client loses its connection to the daemon (phone loses service, or daemon doesn't respond, etc.), the UI displays the "synced" state with green check marks at whatever height the connection is lost:
When the client regains connection, the "synced" state remains in the UI, but the displayed height continues jumping forward (the client resumes scanning even though it says "synced"). Also note the "GIVE" button is clickable at this point, which could present other issues I didn't look into:
This PR's fix
If the client loses its connection to the daemon, after a bit of a delay, the client will re-enter the "Connecting..." state:
Upon re-connecting to the daemon, it re-enters the "Scanning" state:
Important note
This PR pairs with this change to wallet2 on the master branch. That change ensures
refresh()does not mistakenly set the daemon's height to the scanned blockhain height when the daemon returns a failed a response.I first noticed this general UI issue when observing behavior background scanning from 0 via 3rd party remote nodes. Turns out 3rd party remote node responses can be super flaky.