Set progressbar progress according to downloaded blocks since last startup

[fuss132]

With this commit the progress bar in the statusbar updates according to how many blocks are left to sync subtracted by the last block number that was already loaded previously.

I would say this makes more sense, as a user that already loaded 100% of the blockchain before and then reopens his wallet a few hours later would get a progress bar at 99% completion where the missing 1% represents the time he has to wait until the sync completion. With this method progress bar always loads from 0% to 100% evenly.

An example during sync:

When the wallet gets closed and reopened the progressbar starts back at 0% while still being at the lower remaining block count

[brakmic]

LGTM Will wait for colleagues to look into PR as well! Many thanks 👍

[deeponion]

Nice work.... but I have a question: suppose we have a total of 10000 blocks, and we starts at 8000, so it starts at 0%, what if we were on a forked chain and swing to the correct one, and 1000 blocks become orphan, we starts at 7000, now do we start at 0%? or -50%? This could cause confusion. Though it is not a likely scenario, fork in small distance then back can occur from time to time...

[fuss132]

Good point! I don't know how the progressbar would react to a negative value and could maybe even crash. The easiest fix would be to check if a negative sum occurs to detect the scenario that you described and then update the nBlocksAtStartup. I will think about it and add an additional commit to this pr.

[fuss132]

I've added an additional commit which includes the mentioned fix. If in any case nBlocksAtStartup would be greater than nTotalBlocks (which would lead to wrong calculation of the progress bar status) nBlocksAtStartup is set to the current nTotalBlocks. So in the fork scenario we would use the last valid block as starting point

[deeponion]

The change at bitcoingui.cpp, line 631 seems a bit weird to me float nPercentageDone = (count - nBlocksAtStartup) / (nTotalBlocks * 0.01f) - nBlocksAtStartup; shouldn't it be like this? float nPercentageDone = (count - nBlocksAtStartup) / ((nTotalBlocks - nBlocksAtStartup) * 0.01f)

[fuss132]

You are absolutely correct, I fixed this. The error in the calculation is relatively small at the beginning of a sync when nTotalBlocks is way bigger than nBlocksAtStartup, that's why it went unnoticed

[deeponion]

Thanks. I have another question: int newNumBlocksOfPeers = getNumBlocksOfPeers(); int nBlocksAtStartup = getNumBlocksAtStartup(); ... how do you know newNumBlocksOfPeers > nBlocksAtStartup? What if newNumBlocksOfPeers < nBlocksAtStartup? is that possible and handled?

[fuss132]

This: int newNumBlocksOfPeers = getNumBlocksOfPeers(); Has not been changed and is still working as before this commit, it does not influence the progress calculation (only based on nBlocksAtStartup and nTotalBlocks, made sure that nBlocksAtStartup can't be bigger) so there is no issue caused by that possible. If you are asking what could happen in this situation in general not related to this commit I can't tell you that, I would need to check it. As there hasn't been an issue previously I don't think this would cause future problems

Edit: I'll revert my statement for now, I will double check again just to make sure. Please wait with merge before I had a second look at this

[deeponion]

The progress bar pull-request I think I will decline now. In testnet testing we've been doing, the sync'ed block often jump around, so with a relative stable overall percentage seems more reasonable at this time

[fuss132]

No problem. But please in the future please tell me your concerns right away. It's no problem if you don't like an idea but if you can tell me that on the first comment of the pull request I don't have to make several adjustments until you reject it for a reason that was valid from the start