Softforks unit by CodeShark · Pull Request #6747 · bitcoin/bitcoin

CodeShark · 2015-10-02T05:36:23Z

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

dcousens · 2015-10-02T05:44:00Z

concept ACK, once over utACK

luke-jr · 2015-10-02T06:00:28Z

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

CodeShark · 2015-10-02T06:05:21Z

@luke-jr yes.

jonasschnelli · 2015-10-02T06:44:50Z

Nice!
utACK.

btcdrak · 2015-10-02T07:40:51Z

Concept ACK

TheBlueMatt · 2015-10-02T09:33:24Z

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).

CodeShark · 2015-10-02T09:35:54Z

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

TheBlueMatt · 2015-10-02T09:38:25Z

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.

btcdrak · 2015-10-04T07:11:29Z

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

CodeShark · 2015-10-07T14:30:15Z

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

jtimon · 2015-10-11T09:41:16Z

Apart from my nits on #6774, utACK

jtimon · 2015-10-11T09:44:32Z

src/main.cpp

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

jtimon · 2015-10-11T09:48:41Z

I liked '''Consensus::GetFlags''' see jtimon@9ba7d06 even more, but this is definitely a step in the right direction...

dcousens · 2015-10-13T23:30:37Z

re-ACK

jtimon · 2015-10-15T11:38:02Z

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

jtimon · 2015-10-15T11:40:31Z

src/consensus/softforks.h

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.

jtimon · 2016-01-03T23:03:00Z

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

…ion() and SoftForks::IsActive() functions to the softforks unit.

…s::IsSuperMajority directly.

…uperMajority() directly in main.cpp

… it from the unit interface.

jtimon · 2016-03-16T18:10:16Z

rebase or close for now?

btcdrak · 2016-03-16T18:13:58Z

I think 7575 supersedes this.

jtimon · 2016-03-16T19:03:55Z

@btcdrak well, it would need to be greatly rewritten after #7575, but what I think is the general idea it's still possible, see jtimon@dd446fa and jtimon@0321823
But yeah, probably a new PR would be a better idea if it's going to be rewritten.

maflcko · 2016-03-25T17:39:53Z

Needs rebase/new pull if still relevant

laanwj · 2016-03-28T08:09:08Z

Closing for now

CodeShark force-pushed the softforks_unit branch from 7ac4bee to e4d6cab Compare October 2, 2015 06:31

CodeShark force-pushed the softforks_unit branch 2 times, most recently from 2ddd2c8 to 1b00ad2 Compare October 2, 2015 11:39

CodeShark force-pushed the softforks_unit branch from 1b00ad2 to 1b82bc4 Compare October 5, 2015 09:40

laanwj added the Consensus label Oct 5, 2015

CodeShark mentioned this pull request Oct 7, 2015

CHECKLOCKTIMEVERIFY (BIP65) IsSuperMajority() soft-fork #6351

Merged

CodeShark force-pushed the softforks_unit branch from 1b82bc4 to fb12440 Compare October 7, 2015 14:29

CodeShark force-pushed the softforks_unit branch 5 times, most recently from 3b067af to a77e1a9 Compare October 9, 2015 01:39

jtimon reviewed Oct 11, 2015
View reviewed changes

src/main.cpp Outdated

Copy link

Contributor

jtimon Oct 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

jtimon reviewed Oct 15, 2015
View reviewed changes

jtimon mentioned this pull request Jan 3, 2016

IsSuperMajority() moved to separate soft forks unit #6774

Closed

CodeShark added 3 commits January 3, 2016 20:43

Created a SoftForks unit containing only IsSuperMajority() function.

f82a783

Calling IsSuperMajority() inside SoftForks unit in main.cpp

cad891b

Removed IsSuperMajority() from main.cpp

2347ba5

CodeShark force-pushed the softforks_unit branch 2 times, most recently from 3625715 to 47c6f9c Compare January 4, 2016 05:39

CodeShark added 4 commits January 3, 2016 22:29

Added soft fork rules for existing BIPs and SoftForks::CheckBlockVers…

b9aa4f7

…ion() and SoftForks::IsActive() functions to the softforks unit.

Calling SoftForks::IsActive() in main.cpp instead of calling SoftFork…

0b2a516

…s::IsSuperMajority directly.

Call SoftForks::CheckBlockVersion() instead of calling SoftForks::IsS…

08591fe

…uperMajority() directly in main.cpp

Made IsSuperMajority() a static function of softforks.cpp and removed…

2b8466d

… it from the unit interface.

CodeShark force-pushed the softforks_unit branch from 47c6f9c to 2b8466d Compare January 4, 2016 06:30

laanwj closed this Mar 28, 2016

jtimon mentioned this pull request May 19, 2017

Consensus: Introduce static GetScriptFlags (mostly MOVEONLY) #10427

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Conversation

CodeShark commented Oct 2, 2015

SoftForks Unit

Uh oh!

dcousens commented Oct 2, 2015

Uh oh!

luke-jr commented Oct 2, 2015

Uh oh!

CodeShark commented Oct 2, 2015

Uh oh!

jonasschnelli commented Oct 2, 2015

Uh oh!

btcdrak commented Oct 2, 2015

Uh oh!

TheBlueMatt commented Oct 2, 2015

Uh oh!

CodeShark commented Oct 2, 2015

Uh oh!

TheBlueMatt commented Oct 2, 2015

Uh oh!

btcdrak commented Oct 4, 2015

Uh oh!

CodeShark commented Oct 7, 2015

Uh oh!

jtimon commented Oct 11, 2015

Uh oh!

jtimon Oct 11, 2015

Choose a reason for hiding this comment

Uh oh!

jtimon commented Oct 11, 2015

Uh oh!

dcousens commented Oct 13, 2015

Uh oh!

jtimon commented Oct 15, 2015

Uh oh!

jtimon Oct 15, 2015

Choose a reason for hiding this comment

Uh oh!

dcousens Oct 15, 2015

Choose a reason for hiding this comment

Uh oh!

jtimon commented Jan 3, 2016

Uh oh!

jtimon commented Mar 16, 2016

Uh oh!

btcdrak commented Mar 16, 2016

Uh oh!

jtimon commented Mar 16, 2016

Uh oh!

maflcko commented Mar 25, 2016

Uh oh!

laanwj commented Mar 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants