Softforks unit #6747

pull CodeShark wants to merge 7 commits into bitcoin:master from CodeShark:softforks_unit changing 4 files +147 −67
  1. CodeShark commented at 5:36 AM on October 2, 2015: contributor

    SoftForks Unit

    This PR gathers the various soft fork mechanisms into one place making it more orderly. As a positive side effect, it will also make the VersionBits implementation simpler.

    This PR depends on #6774

  2. dcousens commented at 5:44 AM on October 2, 2015: contributor

    concept ACK, once over utACK

  3. luke-jr commented at 6:00 AM on October 2, 2015: member

    Can you put softforks.{cpp,h} under the consensus/ directory?

  4. CodeShark commented at 6:05 AM on October 2, 2015: contributor

    @luke-jr yes.

  5. CodeShark force-pushed on Oct 2, 2015
  6. jonasschnelli commented at 6:44 AM on October 2, 2015: contributor

    Nice! utACK.

  7. btcdrak commented at 7:40 AM on October 2, 2015: contributor

    Concept ACK

  8. TheBlueMatt commented at 9:33 AM on October 2, 2015: member

    Long-term isnt the goal to just switch enforcement of most softforks to blockheight/timestamp and then use versionbits globally? (I believe most things were rolled out via a IsSuperMajority-like system, and then the code was just simplified later by replacing the checks with what-the-switchover-date-was checks).

  9. CodeShark commented at 9:35 AM on October 2, 2015: contributor

    @TheBlueMatt indeed - that's exactly what will end up happening once things are buried deeply enough.

  10. TheBlueMatt commented at 9:38 AM on October 2, 2015: member

    Mmm, ok, well I hadnt really read the code to begin with and I figured you were jumping the gun, but the code does simplify things on its own, so concept ack that.

  11. CodeShark force-pushed on Oct 2, 2015
  12. CodeShark force-pushed on Oct 2, 2015
  13. btcdrak commented at 7:11 AM on October 4, 2015: contributor

    I checked consensus is not affected by spinning up a new node and syncing from scratch. The node synced to the correct tip @ block 377385, 0000000000000000056d10c5f1cc9961ce2476da818a2fb4dd54ca3bcfb99a8d

    ACK

  14. CodeShark force-pushed on Oct 5, 2015
  15. laanwj added the label Consensus on Oct 5, 2015
  16. CodeShark force-pushed on Oct 7, 2015
  17. CodeShark commented at 2:30 PM on October 7, 2015: contributor

    Rebased atop #6774 to make it easier to review code movements.

  18. CodeShark force-pushed on Oct 7, 2015
  19. CodeShark force-pushed on Oct 9, 2015
  20. CodeShark force-pushed on Oct 9, 2015
  21. CodeShark force-pushed on Oct 9, 2015
  22. CodeShark force-pushed on Oct 9, 2015
  23. jtimon commented at 9:41 AM on October 11, 2015: contributor

    Apart from my nits on #6774, utACK

  24. in src/main.cpp:None in a77e1a9b25 outdated 
    1719 | -    // two in the chain that violate it. This prevents exploiting the issue against nodes during their
1720 | -    // initial block download.
1721 | -    bool fEnforceBIP30 = (!pindex->phashBlock) || // Enforce on CreateNewBlock invocations which don't have a hash.
1722 | -                          !((pindex->nHeight==91842 && pindex->GetBlockHash() == uint256S("0x00000000000a4d0a398161ffc163c503763b1f4360639393e0e4c8e300e0caec")) ||
1723 | -                           (pindex->nHeight==91880 && pindex->GetBlockHash() == uint256S("0x00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721")));
1724 | +    // For soft fork details, look in consensus/softforks.cpp
    jtimon commented at 9:44 AM on October 11, 2015:

    Are these "look in consensus/softfork.cpp" comments really necessary or useful?

  25. jtimon commented at 9:48 AM on October 11, 2015: contributor

    I liked '''Consensus::GetFlags''' see https://github.com/jtimon/bitcoin/commit/9ba7d0641019d7f6c08f7b15ce3a2bd2e5e8a4ac even more, but this is definitely a step in the right direction...

  26. dcousens commented at 11:30 PM on October 13, 2015: contributor

    re-ACK

  27. jtimon commented at 11:38 AM on October 15, 2015: contributor

    Wouldn't it make sense to rebase this on top of #6816 ?

  28. in src/consensus/softforks.h:None in a77e1a9b25 outdated 
      18 | +    BIP34,
  19 | +    BIP42,
  20 | +    BIP62,
  21 | +    BIP65,
  22 | +    BIP66,
  23 | +    BIP68,
    jtimon commented at 11:40 AM on October 15, 2015:

    In my optinion there's no need to include future softforks/hardforks here (perhaps with the exception of BIP65 which is already merged as a policy rule), they can add themselves in their own PRs if they're merged after this.

    dcousens commented at 11:42 AM on October 15, 2015:

    Agreed

  29. jtimon commented at 11:03 PM on January 3, 2016: contributor

    Sorry, I meant to say the following here: #6774 (comment)

  30. Created a SoftForks unit containing only IsSuperMajority() function. f82a7831a4
  31. Calling IsSuperMajority() inside SoftForks unit in main.cpp cad891bca4
  32. Removed IsSuperMajority() from main.cpp 2347ba52a8
  33. CodeShark force-pushed on Jan 4, 2016
  34. CodeShark force-pushed on Jan 4, 2016
  35. Added soft fork rules for existing BIPs and SoftForks::CheckBlockVersion() and SoftForks::IsActive() functions to the softforks unit. b9aa4f7bb5
  36. Calling SoftForks::IsActive() in main.cpp instead of calling SoftForks::IsSuperMajority directly. 0b2a516fce
  37. Call SoftForks::CheckBlockVersion() instead of calling SoftForks::IsSuperMajority() directly in main.cpp 08591fe8c8
  38. Made IsSuperMajority() a static function of softforks.cpp and removed it from the unit interface. 2b8466d201
  39. CodeShark force-pushed on Jan 4, 2016
  40. jtimon commented at 6:10 PM on March 16, 2016: contributor

    rebase or close for now?

  41. btcdrak commented at 6:13 PM on March 16, 2016: contributor

    I think 7575 supersedes this.

  42. jtimon commented at 7:03 PM on March 16, 2016: contributor

    @btcdrak well, it would need to be greatly rewritten after #7575, but what I think is the general idea it's still possible, see https://github.com/jtimon/bitcoin/commit/dd446faf641350fb469865fc3586d1e088bdfb00 and https://github.com/jtimon/bitcoin/commit/032182336d84a420f33be3ab7d204c9b5f8ae8cf But yeah, probably a new PR would be a better idea if it's going to be rewritten.

  43. MarcoFalke commented at 5:39 PM on March 25, 2016: member

    Needs rebase/new pull if still relevant

  44. laanwj commented at 8:09 AM on March 28, 2016: member

    Closing for now

  45. laanwj closed this on Mar 28, 2016
  46. MarcoFalke locked this on Sep 8, 2021
Contributors
CodeSharkdcousensluke-jrjonasschnellibtcdrakTheBlueMattjtimonMarcoFalkelaanwj
Labels
Consensus
Linked (view graph)
#6774 IsSuperMajority() moved to separate soft forks unit#10427 Consensus: Introduce static GetScriptFlags (mostly MOVEONLY)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-24 18:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me