VerifyDB progress #4223

pull cozz wants to merge 1 commits into bitcoin:master from cozz:cozz7 changing 10 files +74 −5

cozz commented at 5:01 PM on May 23, 2014: contributor
laanwj commented at 9:23 AM on May 24, 2014: member

Nice!
laanwj commented at 6:46 AM on May 29, 2014: member

Tested this with various checkblocks and checklevel values (including 0) and it worked for me. ACK
in src/rpcblockchain.cpp:None in 090bd9c675 outdated
```
 427 | @@ -427,7 +428,11 @@ Value verifychain(const Array& params, bool fHelp)
 428 |      if (params.size() > 1)
 429 |          nCheckDepth = params[1].get_int();
 430 |  
 431 | -    return VerifyDB(nCheckLevel, nCheckDepth);
 432 | +    uiInterface.ShowProgress(_("Verifying blocks..."), 0);
```
sipa commented at 10:29 AM on May 29, 2014:

Not very clean that the different calls with uiInterface.ShowProgress("Verifying blocks..."... are called from two very different places.

laanwj commented at 10:36 AM on May 29, 2014:

Right - would be better to have this all contained to VerifyDB.

cozz commented at 12:59 PM on May 29, 2014:

Yes not so clean, I did that because there are a lot of "return" statements in VerifyDB(..), so I would need to add a signal before every "return" in VerifyDB(..). I will change it.

laanwj commented at 1:14 PM on May 29, 2014:

Just create a a class "VerifyingBlocks" and use RAII. Every return path will hit the destructor, so do the signal there.
cozz commented at 2:48 PM on May 29, 2014: contributor
update:
- created a RAII wrapper for VerifyDB
sipa commented at 2:58 PM on May 29, 2014: member

ACK core changes.
in src/main.h:None in 58d8614aa8 outdated
```
1027 | +class CVerifyDB {
1028 | +public:
1029 | +
1030 | +    CVerifyDB(int nCheckLevelIn, int nCheckDepthIn)
1031 | +    {
1032 | +        nCheckLevel = nCheckLevelIn;
```
laanwj commented at 3:13 PM on May 29, 2014:

I'd prefer moving all the implementation to main.cpp, instead of specifying it in the header. This also avoids having to make main.h depend on ui_interface.h

sipa commented at 3:30 PM on May 29, 2014:

Agree.
cozz commented at 4:46 PM on May 29, 2014: contributor
update:
- moved implementation to main.cpp
in src/main.cpp:None in f56e3cc9bc outdated
```
2967 | @@ -2968,7 +2968,17 @@ bool static LoadBlockIndexDB()
2968 |      return true;
2969 |  }
2970 |  
2971 | -bool VerifyDB(int nCheckLevel, int nCheckDepth)
2972 | +void CVerifyDB::Init()
```
brandondahler commented at 3:41 AM on May 30, 2014:

Nit: Why use Init() and Finialize() instead of CVerifyDB() and ~CVerifyDB()?

laanwj commented at 6:30 AM on May 30, 2014:

Agree with @brandondahler here. Those methods are redundant.
cozz commented at 8:55 PM on May 30, 2014: contributor
update:
- moved con/destructor to main.cpp and removed init and finalize
VerifyDB progress 06a91d9698
in src/qt/clientmodel.cpp:None in 91bdd686aa outdated
```
 198 | @@ -198,7 +199,37 @@ QString ClientModel::formatClientStartupTime() const
 199 |      return QDateTime::fromTime_t(nClientStartupTime).toString();
 200 |  }
 201 |  
 202 | +void ClientModel::showProgress(const QString &title, int nProgress)
```
laanwj commented at 2:24 PM on June 2, 2014:

Displaying a dialog box in the model code is a bit questionable - that's supposed to happen in some view - the model should only pass on or process signals from the core.

Diapolo commented at 3:54 PM on June 2, 2014:

I remember I got that feedback too once :). And I remember I decided you were right.
cozz commented at 1:41 PM on June 3, 2014: contributor
update:
- moved dialogbox to bitcoingui
BitcoinPullTester commented at 2:28 PM on June 3, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/06a91d9698762fe56fca3bd33484bddc9f020405 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
sipa commented at 7:58 PM on June 3, 2014: member

ACK
laanwj merged this on Jun 4, 2014
laanwj closed this on Jun 4, 2014
laanwj referenced this in commit 358a61ee06 on Jun 4, 2014
DrahtBot locked this on Sep 8, 2021

Contributors