IsSuperMajority() moved to separate soft forks unit #6774

1. 
   CodeShark commented at 2:22 pm on October 7, 2015: contributor

   CODE MOVE ONLY

   This PR creates a new unit which will be handling soft fork logic. For now all we do is move IsSuperMajority() from main.cpp to consensus/softforks.cpp.

2. CodeShark force-pushed on Oct 7, 2015
3. 
   dcousens commented at 0:40 am on October 8, 2015: contributor

   utACK
4. CodeShark renamed this:
   ISM to softforks unit
   IsSuperMajority() moved to separate soft forks unit
   on Oct 8, 2015
5. 
   jonasschnelli commented at 9:16 am on October 8, 2015: contributor

   utACK
6. 
   paveljanik commented at 9:27 am on October 8, 2015: contributor

   Please clean up class/struct Params:

   0src pavel$ grep "[class|strict] Params" consensus/*h
   1consensus/params.h:struct Params {
   2consensus/softforks.h:class Params;
   3src pavel$ 
   

7. in src/consensus/softforks.h: in 539702dea9 outdated

    6+#define BITCOIN_SOFTFORKS_H
    7+
    8+class CBlockIndex;
    9+namespace Consensus {
   10+
   11+class Params;
   

   ---

   ---

   

   dcousens commented at 9:29 am on October 8, 2015:

   As pointed out by @paveljanik, this should be a struct.
8. laanwj added the label Refactoring on Oct 8, 2015
9. CodeShark force-pushed on Oct 9, 2015
10. Created a SoftForks unit containing only IsSuperMajority() function. bfbba3952f
11. Calling IsSuperMajority() inside SoftForks unit in main.cpp 4dea8e3cbc
12. Removed IsSuperMajority() from main.cpp df089cf1f6
13. 
    CodeShark commented at 1:14 am on October 9, 2015: contributor

    @paveljanik @dcousens Fixed class->struct
14. in src/Makefile.am: in df089cf1f6

     98@@ -99,6 +99,7 @@ BITCOIN_CORE_H = \
     99   compressor.h \
    100   consensus/consensus.h \
    101   consensus/params.h \
    102+  consensus/softforks.h \
    

    ---

    ---

    

    jtimon commented at 9:37 am on October 11, 2015:

    If we’re going to eventually use this for Hardfork consensus changes too, maybe it would be better to have a more generic name for the file/module: forks, changes, upgrades?

    ---

    

    dcousens commented at 11:56 pm on October 13, 2015:

    @jtimon could be done later

    ---

    

    jtimon commented at 11:32 am on October 20, 2015:

    Or could be done now with less total work, that’s the point.
15. in src/consensus/softforks.cpp: in df089cf1f6

    0@@ -0,0 +1,25 @@
    1+// Copyright (c) 2009-2015 Eric Lombrozo, The Bitcoin Core developers
    2+// Distributed under the MIT software license, see the accompanying
    3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
    4+
    5+#include "chain.h"
    6+#include "consensus/params.h"
    7+#include "softforks.h"
    8+
    9+using namespace Consensus::SoftForks;
    

    ---

    ---

    

    jtimon commented at 9:38 am on October 11, 2015:

    The additional namespace Softforks seems unnecessarily verbose, just Consensus seems enough.
16. 
    jtimon commented at 9:38 am on October 11, 2015: contributor

    Apart from my bike-shedding nits, ut ACK.
17. 
    jtimon commented at 7:56 pm on January 3, 2016: contributor

    I changed my mind about this. I prefer something like https://github.com/jtimon/bitcoin/commit/8ec82416cdb48f6f01d49688406918be864d80b2 again. NACK In any case rebase or close?
18. 
    dcousens commented at 10:47 pm on January 3, 2016: contributor

    @jtimon why do you prefer it?

    Still utACK from me, but needs rebase

19. 
    jtimon commented at 10:53 pm on January 3, 2016: contributor

    It’s simpler and achieves the task of encapsulating consensus code more simply (although of course this is relative). I don’t think a UseRule() function is interesting getting the consensus validation flags that include those rules (and I would unify other future consensus flags [like the one for bip113] with the script ones too).
20. 
    jtimon commented at 10:59 pm on January 3, 2016: contributor

    Sorry, that was a mistake, I thought I was talking on #6747 . This is just a tiny and pointless moveonly IMO. Instead of this, I would prefer something like this https://github.com/jtimon/bitcoin/commit/f8c34f27d4020880647cbdbafad70793882cef79 (see https://github.com/jtimon/bitcoin/commits/libconsensus-f2 for the “big picture” and #7091 for the pre-document-with-words-and-pictures opened first step).
21. 
    CodeShark commented at 4:14 am on January 4, 2016: contributor

    @dcousens Rebasing - will push shortly. #6747 that is, not this one.
22. 
    btcdrak commented at 2:30 pm on April 5, 2016: contributor

    I think we can close this now. Also I recall talk of replacing the ISM checks with block heights.
23. laanwj closed this on Apr 6, 2016

    ---

24. MarcoFalke locked this on Sep 8, 2021

Contributors
CodeShark dcousens jonasschnelli paveljanik jtimon btcdrak

Labels
Refactoring