Move IsInitialBlockDownload from main to checkpoints #3999
Conversation
Rebased-By: Wladimir J. van der Laan <laanwj@gmail.com>
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4b0eb790681673e169c8a421150ddac4b0a949a9 for binaries and test log. |
| } | ||
|
|
||
|
|
|
I don't see why it is either useful or wanted to have that method there. It doesn't belong there. |
|
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. |
|
@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). |
|
Okay, never mind, didn't know this was so controversial. No one commented this on #3461. |
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.