Bugfix: Don’t check the genesis block header before accepting it #6299

1. 
   jtimon commented at 7:30 pm on June 17, 2015: contributor

   This should fix an error that was introduced in #5975 , thanks @sdaftuar for reporting the error. I will work on a more elegant solution: the genesis block should never be checked at all; it is valid by definition. But it seems that will be more work than I first thought so let’s just fix the bug first.
2. in src/main.cpp: in 460c57e049 outdated

   2808@@ -2809,6 +2809,7 @@ bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, CBloc
   2809     uint256 hash = block.GetHash();
   2810     BlockMap::iterator miSelf = mapBlockIndex.find(hash);
   2811     CBlockIndex *pindex = NULL;
   2812+    if (hash != chainparams.GetConsensus().hashGenesisBlock) {
   2813     if (miSelf != mapBlockIndex.end()) {
   

   ---

   ---

   

   laanwj commented at 7:34 pm on June 17, 2015:

   Indentation missing

   ---

   

   jtimon commented at 7:41 pm on June 17, 2015:

   yep, I want to avoid indenting the whole block to reduce the diff and potential for merge conflicts. After all, at some point we will start running clang-format project wise right before forking for major releases. Is that still the plan @laanwj @sipa ?

   ---

   

   petertodd commented at 3:17 am on June 18, 2015:

   I’d prefer those merge conflicts to stay merge conflicts, as it forces you to understand if that additional check matters for your merge.

   What can I say, I once fixed a bug where a lack of indentation of a newly added bit of code lead to a mistake that could have potentially played a part of getting someone killed, so… :/

   ---

   

   jtimon commented at 7:29 am on June 18, 2015:

   I’m happy to indent if people think it’s better.

   ---

   

   laanwj commented at 9:23 am on June 18, 2015:

   Yes, that is still the plan at some point, although I’d say it’s very low priority.

   Reducing the diff is generally a good reason, but not enough to deviate this much from the coding style IMO. This makes the control flow harder to interpret in an important function.

3. laanwj added the label Bug on Jun 17, 2015
4. 
   laanwj commented at 7:37 pm on June 17, 2015: member

   Can you please cherry-pick #6298 into this, to see if the reindex test passes in travis.
5. 
   jtimon commented at 7:46 pm on June 17, 2015: contributor

   @laanwj Incorporated #6298
6. jtimon force-pushed on Jun 18, 2015
7. 
   jtimon commented at 9:36 am on June 18, 2015: contributor

   Updated with the correct indentation (bigger diff).
8. 
   laanwj commented at 12:36 pm on June 18, 2015: member

   utACK
9. 
   theuni commented at 5:27 pm on June 19, 2015: member

   Btw, for the sake of easier review, you can always do a diff that ignores whitespace via git diff -w.

   To see that at github, just append ?w=1 to the url: https://github.com/bitcoin/bitcoin/pull/6299/files?w=1

10. Bugfix: Don't check the genesis block header before accepting it

    This fixes an error triggered when running with -reindex after #5975

    36c97b4e5d
11. test: Move reindex test to standard tests

    This test finishes very quickly, so it should be part of the default set
    of tests in rpc-tests.

    4f40716dcb
12. jtimon force-pushed on Jun 20, 2015
13. 
    jtimon commented at 11:26 pm on June 20, 2015: contributor

    Needed rebase. @theuni Thanks, that’s useful.
14. 
    sdaftuar commented at 5:12 pm on June 24, 2015: member

    Tested ACK
15. jtimon referenced this in commit f629538693 on Jun 26, 2015
16. 
    jtimon commented at 10:25 am on June 26, 2015: contributor

    There’s an alternative solution to the bug in #6230.
17. laanwj merged this on Jun 26, 2015
18. laanwj closed this on Jun 26, 2015

    ---

19. laanwj referenced this in commit 24f24896d6 on Jun 26, 2015
20. 
    laanwj commented at 1:41 pm on June 26, 2015: member

    ACK
21. jtimon referenced this in commit d84dc4771b on Jun 26, 2015
22. jtimon referenced this in commit 60dbe9a6d6 on Jul 1, 2015
23. jtimon referenced this in commit ccbf23b72d on Jul 1, 2015
24. zkbot referenced this in commit 9af55822fb on Feb 15, 2017
25. zkbot referenced this in commit df07f9ad23 on Feb 15, 2017
26. zkbot referenced this in commit a7cf698873 on Mar 4, 2017
27. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon laanwj petertodd theuni sdaftuar

Labels
Bug