Move IsInitialBlockDownload from main to checkpoints #3999

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2014_04_iniitalblockdl_to_checkpoints changing 6 files +34 −29

laanwj commented at 12:59 PM on April 3, 2014: member

This is the first commit of "Don't use third-party "what is my IP" services (rebase)" #3461.

It is independent from the functionality in that pull and makes it easier to keep that it up to date. Also I think the code move makes sense, and it helps a little bit in reducing the size of main.cpp.
Move IsInitialBlockDownload from main to checkpoints
Rebased-By: Wladimir J. van der Laan <laanwj@gmail.com>
4b0eb79068
BitcoinPullTester commented at 1:51 PM on April 3, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4b0eb790681673e169c8a421150ddac4b0a949a9 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.

in src/checkpoints.h:None in 4b0eb79068

  26 | @@ -27,6 +27,10 @@
  27 |      double GuessVerificationProgress(CBlockIndex *pindex, bool fSigchecks = true);
  28 |  
  29 |      extern bool fEnabled;
  30 | +
  31 | +    /** Check whether we are doing an initial block download (synchronizing from disk or network) */
  32 | +    bool IsInitialBlockDownload();
  33 |  }
  34 |  
  35 | +

Diapolo commented at 5:04 PM on April 3, 2014:

Nit: n/t ;)...

sipa commented at 5:42 PM on April 3, 2014: member

I don't see why it is either useful or wanted to have that method there. It doesn't belong there.
sipa commented at 5:44 PM on April 3, 2014: member

Reducing the size of main is a good idea, but not by moving logic to arbitrary places.

All variables accessed by this method are internal state to main that shouldn't be exposed in the first place, and only is because of legacy reasons.
sipa commented at 5:59 PM on April 3, 2014: member

@gmaxwell Just told me he originally did this to be able to access IsInitialBlockDownload() from net.

I still think this is a bad idea. It accesses fImporting, fReindex, chainActive, local time, ... none of which have anything to do with checkpoints. Also, it requires locking cs_main before calling because of that, making it illegal to just call from network code anyway.

The right way is adding a wrapper method in main that acquires cs_main and then calls the current implementation, and add that method to CNodeSignals. That way, net can safely call it.

The long term solution is merging net and main, and then separating the "validation logic" and the "peer state logic" to different modules. IsInitialBlockDownload would become part of the second.

Also, and independent from this: I think checkpoints should move into chainparams, and become data-only (or just disappear).
laanwj commented at 6:27 AM on April 4, 2014: member

Okay, never mind, didn't know this was so controversial. No one commented this on #3461.
laanwj closed this on Apr 4, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#3461 Don't use third-party "what is my IP" services (rebase)