This is less confusing to most people, and doesn't rely on estimates of the total number of blocks received from other nodes.
Like in the Android app, in the progress bar label now shows the time that the node is behind instead of the number of blocks left.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ab823efbf0a51e0f8dab1ea3d1f27552fea12332 for binaries and test log.
This change seems to require some small re-translations, is this intended for 0.8 or for 0.8.x? As translators need time, we should be sure when to pull this. I don't want to argue over the pull itself ^^, didn't yet try it anyway.
I would also feel better, if the testnet todo could be fixed before pulling it.
@laanwj If you want to mimic the behaviour of Bitcoin Wallet, you should have a look at https://code.google.com/p/bitcoin-wallet/source/browse/wallet/src/de/schildbach/wallet/ui/BlockchainStateFragment.java (lines 145-169).
First of all, it does not show seconds or minutes. It starts with "1 hour behind."
1-48 hours: x hours behind 2-14 days: x days behind 2-xx weeks: x weeks behind
I'm thinking of even allowing months.
It starts with "1 hour behind."
It's important to not start reporting as soon as an hour. Otherwise users start freaking out and corrupting their installs whenever there is a 1 hour block gap (plus the timestamps can pretty easily be an hour behind).
Diapolo: At first I simply re-used the "XXX ago" messages, but I think it was a bit confusing and "behind" indicates better what is the case. Yeah it will cause some translations.
I'm not sure about testnet. I suppose it's not really that important what date is used there for the genesis block.
Schildbach: I don't really like to use weeks or months, it's very hard to see progress that way. Seconds are never shown, as it starts showing the progress bar at 90 minutes behind. It may be that showing only whole hours is better though, showing minutes pretends to be too precise.
@gmaxwell I had these thoughts as well about 18 months ago. Back then I was seeing max 45 minutes gap, and I thought timestamps are not off more than 15 minutes. Note that I have a separate warning for when your system clock is off.
So how many minutes of threshold would you suggest?
@laanwj Its probably shed-painting, but people start to think in weeks, months or even years if you have larger timespans. Nobody has got a feel of how much 1237 days actually are.
Ok but apart from small details, perfect being the enemy of good and such, is this a step forward compared to the current implementation?
I agree with @schildbach that when numbers get too large, they become hard to interpret. I wonder if using more decimals just to show progress would be useful. For example: X hours (2-48 hours), X.X days (2-14 days), X.XX weeks (14-60 days), X.XXX months (>60 days). Certainly the decimals don't really convey any actual information anymore, but as a whole it's easy to interpret and does show progress.
Decimals imo have the same problems as large numbers, it's still a garble of numbers you're just adding a point somewhere in the middle.
Another option would be to convey "liveness" in another way than global progress so that it doesn't matter that large time units are used.
For example a lowtech way would be to add/remove a dot to the label for every N blocks that comes in, or an icon that animates one step,etc...
I'll give that a try...
Ok I changed the time units to @schildbach's, and for the first time in history the spinner does something useful: it spins a frame every time setNumberOfBlocks is called with a changed number of blocks. So both liveness and progress is covered again.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/3683d71a8535c2e2505799baf2e7a1f7a1aaf3aa for binaries and test log.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/75124792ad76204a93c0b2e97c054dda618163bc for binaries and test log.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/876fa2cbae0cb990d79bc981ccbabccdc55f9ee1 for binaries and test log.
56 | } 57 | 58 | +QDateTime ClientModel::getGenesisBlockDate() const 59 | +{ 60 | + /// TODO testnet 61 | + return QDateTime::fromTime_t(1231006505); // Genesis block's time
I think this constant belongs either in either main or checkpoints - the distinction between mainnet and testnet could also happen there automatically.
Agreed, that would be a better place.
Here is a commit that changes the progressbar logic to use (projected & corrected) transaction counts for progress: https://github.com/sipa/bitcoin/commit/e1882ae47afe2e05e627f34af885a910c1303fc2
Cool, so I understand that makes the progressbar better match the part of total time spent acquiring the block chain?
@laanwj Yes, it actually seems to speed up a bit, as the very early blocks have almost no transactions in them, but still require a per-block processing time.
Only disadvantage is that it requires 4 measured/guessed control parameters. Guessing them wrong doesn't result in worse behaviour than we already have, but it does require some extra work when adding a checkpoint.
With the current code in the pull request, usig secs, it looks like the first half-ish goes really fast, and after that it almost grinds to a halt (but that was the same when still using blocks).
Yes, this fixes that very nicely. I mean this almost reverts it: it seems the very first "blocks" go very slowly, and it speeds up slightly along the way. Try it :)
I will once I get home :) was just trying to get the gist of it
Added a commit to improve testnet support: https://github.com/sipa/bitcoin/commits/progressbar
Here's a graph of block-based progress vs my branch's code during a reindex on my VPS: http://bitcoin.sipa.be/progress.png
The last checkpoint is at 0.299252 progress, so slightly before the strange 2-minute gap (i have no clue what happened there, perhaps an I/O hiccup at the hosting provider).
This is less confusing to most people, and doesn't rely on estimates
of the total number of blocks received from other nodes.
Looks good! I've just merged your changes into this pull.
@gavinandresen In the added refactor commit, I've added some comments.
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/100d0c570f2c07c8c8b0993f5c5f69ae1e528270 for binaries and test log.
This could happen for one of several reasons:
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.
Anyone an idea why pulltester fails to compile?
Here's another graph, on a fast 6-core machine. The slowdown factor of 15.0 in the current code is clearly way too much here... http://bitcoin.sipa.be/progress-6core.png
I don't understand the pulltester failure either, it gives a warning then bails out.
Scroll up: checkpoints.cpp:45: error: too many initializers for 'const Checkpoints::CCheckpointData' checkpoints.cpp:53: error: too many initializers for 'const Checkpoints::CCheckpointData'
13 | @@ -14,14 +14,21 @@ 14 | { 15 | typedef std::map<int, uint256> MapCheckpoints; 16 | 17 | - // 18 | + static const double fSigcheckVerificationFactor = 15.0;
I would like to see a comment for this :).
It's how many times transactions after the checkpoint are expected to be slower than those before. Source: me randomly pressing some keys.
21 | @@ -22,6 +22,8 @@ 22 | 23 | // Returns last CBlockIndex* in mapBlockIndex that is a checkpoint 24 | CBlockIndex* GetLastCheckpoint(const std::map<uint256, CBlockIndex*>& mapBlockIndex); 25 | + 26 | + double GuessVerificationProgress(CBlockIndex *pindex);
...and this :)?
Would: // Guess verification progress do?
1 | @@ -2,7 +2,7 @@ 2 | #define GUICONSTANTS_H 3 | 4 | /* Milliseconds between model updates */ 5 | -static const int MODEL_UPDATE_DELAY = 500; 6 | +static const int MODEL_UPDATE_DELAY = 250;
Does this lead to a noticeable higher CPU usage?
No (bitcoin spends its time in block verification, drawing 4 times a second instead of 2 really doesn't matter, maybe it did in the times of commodore 64? :-)
